Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ValueData::Alias #49

Merged
merged 4 commits into from
Aug 21, 2024
Merged

Remove ValueData::Alias #49

merged 4 commits into from
Aug 21, 2024

Conversation

sbillig
Copy link
Collaborator

@sbillig sbillig commented Jul 14, 2024

The new parser's use of aliases revealed some alias-related bugs in some of the optimizations. The cleanest fix for this is to just remove ValueData::Alias, and instead replace the uses of the alias Value with the original Value (which is easy, because the dfg already tracks which instructions use each Value.)

I also changed the parser so that it no longer uses aliases. The prior method of handling values in the parser lead to the wrong type being assigned to some insn result values (in cases where the first argument to some insn was a value whose definition had not yet been seen by the parser). The new method is to first do a pass over all statements in a function to "declare" all values with their proper types (by calling dfg.make_value with dummy ValueData).

@sbillig sbillig marked this pull request as ready for review July 29, 2024 00:38
@sbillig sbillig requested a review from Y-Nak July 29, 2024 00:38
Copy link
Collaborator

@Y-Nak Y-Nak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Y-Nak Y-Nak merged commit fbb42c4 into fe-lang:main Aug 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants