sroa_pass!: Mark statements that were updated #49340
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sroa_pass! can introduce new type information that inference did not see (e.g. when forwarding field accesses through a mutable). Currently, we just accept that in base, but there are external pass pipelines that run multiple rounds of inference and would like to refine these. Make this possible by introducing a new IR flag that sroa_pass! sets when it modifies a statement. There are currently no consumers in the Base pipeline, but the idea is that these can be passed into _ir_abstract_constant_propagation for refinement.
Keno
added a commit
that referenced
this pull request
Jun 13, 2023
This continues the refactoring begun by #49340 to have irinterp consume the IR_FLAG_REFINED flag. This essentially has the same effect as the extra_reprocess bitset that irinterp takes, so we can remove that. However, there is a related issue where we would like to inform irinterp that we have *already* refined the type of a particular statement (likely using information not available to the irinterp) and would like it to just propagate that if possible. So bring back that extra bitset with a new name and these new semantics to make that possible. While I was working on this, I also noticed that the control hook I added in #48199 wasn't quite working as advertised. I don't currently need it, so rather than trying to work through an API without a concrete consumer, just nuke that hook for now. I do still think it'll be required at some point, but we can always add it back.
Keno
added a commit
that referenced
this pull request
Jun 13, 2023
This continues the refactoring begun by #49340 to have irinterp consume the IR_FLAG_REFINED flag. This essentially has the same effect as the extra_reprocess bitset that irinterp takes, so we can remove that. However, there is a related issue where we would like to inform irinterp that we have *already* refined the type of a particular statement (likely using information not available to the irinterp) and would like it to just propagate that if possible. So bring back that extra bitset with a new name and these new semantics to make that possible. While I was working on this, I also noticed that the control hook I added in #48199 wasn't quite working as advertised. I don't currently need it, so rather than trying to work through an API without a concrete consumer, just nuke that hook for now. I do still think it'll be required at some point, but we can always add it back.
Keno
added a commit
that referenced
this pull request
Jun 24, 2023
In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit changes compact prevent compaction if IR_FLAG_REFINED is set on a statement. This is the minimal change to fix this issue, but it is probably suboptimal for the base pipeline, because we (currently) do not do any re-inference (other than during semi-concrete eval), so the flag never gets cleared. I think the viable alternatives are: 1. Be more selective about setting IR_FLAG_REFINED only in situations where there is an actual improvement in the detected type. 2. Have compact look at whether the original IR statement was refined and if so, propagate the flag. 3. Both. However, these are somewhat separate optimizations from fixing the underlying bug (and indepedent from each other), so let's experiment with those in separate commits.
Keno
added a commit
that referenced
this pull request
Jun 24, 2023
In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit changes compact prevent compaction if IR_FLAG_REFINED is set on a statement. This is the minimal change to fix this issue, but it is probably suboptimal for the base pipeline, because we (currently) do not do any re-inference (other than during semi-concrete eval), so the flag never gets cleared. I think the viable alternatives are: 1. Be more selective about setting IR_FLAG_REFINED only in situations where there is an actual improvement in the detected type. 2. Have compact look at whether the original IR statement was refined and if so, propagate the flag. 3. Both. However, these are somewhat separate optimizations from fixing the underlying bug (and indepedent from each other), so let's experiment with those in separate commits.
Keno
added a commit
that referenced
this pull request
Jun 25, 2023
This is a more invasive version of #50281, implementing alternative 2. Because this solves the same problem, the motivation is the same, so quoth much of the commit message: In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit adds a special `Refined` marker in ssa_rename that lets compact propagate the IR_FLAG_REFINED flag to any users of statements that may have been compacted away.
Keno
added a commit
that referenced
this pull request
Jun 25, 2023
This is a more invasive version of #50281, implementing alternative 2. Because this solves the same problem, the motivation is the same, so quoth much of the commit message: In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit adds a special `Refined` marker in ssa_rename that lets compact propagate the IR_FLAG_REFINED flag to any users of statements that may have been compacted away.
Keno
added a commit
that referenced
this pull request
Jun 26, 2023
This is a more invasive version of #50281, implementing alternative 2. Because this solves the same problem, the motivation is the same, so quoth much of the commit message: In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit adds a special `Refined` marker in ssa_rename that lets compact propagate the IR_FLAG_REFINED flag to any users of statements that may have been compacted away.
Keno
added a commit
that referenced
this pull request
Jun 26, 2023
This is a more invasive version of #50281, implementing alternative 2. Because this solves the same problem, the motivation is the same, so quoth much of the commit message: In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit adds a special `Refined` marker in ssa_rename that lets compact propagate the IR_FLAG_REFINED flag to any users of statements that may have been compacted away.
Keno
added a commit
that referenced
this pull request
Jun 26, 2023
This is a more invasive version of #50281, implementing alternative 2. Because this solves the same problem, the motivation is the same, so quoth much of the commit message: In #49340, I added an ir flag (currently only set by sroa) to allow sparse-reinference of ir after optimization passes that may improve type information. However, as currently implemented, this flag gets dropped when the IR is compacted, defeating the purpose of the flag, because it can no longer be reliably used for sparse re-inference. This commit adds a special `Refined` marker in ssa_rename that lets compact propagate the IR_FLAG_REFINED flag to any users of statements that may have been compacted away.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
sroa_pass! can introduce new type information that inference did not see (e.g. when forwarding field accesses through a mutable). Currently, we just accept that in base, but there are external pass pipelines that run multiple rounds of inference and would like to refine these. Make this possible by introducing a new IR flag that sroa_pass! sets when it modifies a statement. There are currently no consumers in the Base pipeline, but the idea is that these can be passed into _ir_abstract_constant_propagation for refinement.