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

Tighter array eltype for renumber_ssa #37499

Merged
merged 1 commit into from
Sep 14, 2020
Merged

Tighter array eltype for renumber_ssa #37499

merged 1 commit into from
Sep 14, 2020

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Sep 9, 2020

Unlike renumber_ssa2 in compiler/ir.jl which actually takes an array with elements of
many different types, all callers of renumber_ssa (and renumber_ssa!) only ever assign
SSAValue to the array. Also no one ever checks isassigned on this array
so a Vector{SSAValue} should work just fine here.

Try to maintain a similar level of error checking by replacing #undef with SSAValue(-1)
(already used by construct_ssa!) and adding an assertion in renumber_ssa
to check for cases that throws UndefVarError previously.

Also removed an unused parameter.

Unlike `renumber_ssa2` in `compiler/ir.jl` which actually takes an array with elements of
many different types, all callers of `renumber_ssa` (and `renumber_ssa!`) only ever assign
`SSAValue` to the array. Also no one ever checks `isassigned` on this array
so a `Vector{SSAValue}` should work just fine here.

Try to maintain a similar level of error checking by replacing `#undef` with `SSAValue(-1)`
(already used by `construct_ssa!`) and adding an assertion in `renumber_ssa`
to check for cases that throws `UndefVarError` previously.

Also removed an unused parameter.
@vtjnash vtjnash merged commit a0a68a5 into master Sep 14, 2020
@vtjnash vtjnash deleted the yyc/typeinf/ssanum branch September 14, 2020 20:06
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