- Notifications
You must be signed in to change notification settings - Fork5.1k
Enable global constant propagation for small types#57726
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Enable global constant propagation for small types#57726
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ghost commentedAug 19, 2021
Tagging subscribers to this area:@JulieLeeMSFT Issue DetailsJust as the title says - remove the restriction that we do not propagate constants for small-typed trees. It is safe because VNadds the necessary truncating casts when evaluating the assignment. Now, small-typed nodes themselves are a bit rare, but one scenario where they come up is in field-by-field copying of promoted structs, which is where practically all the diffs for this change come from. The substantial code here is only in the third commit, the rest is miscellaneous cleanup. Diffs for this change:
|
So that there is one canonical way to checkif a type can represent some value.
808faf5 to97f96ddCompareCC @dotnet/jit-contrib please review the community PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This looks good to me, thanks!
Because this is an impactful change, I think stress testing is warranted for this PR. SPMI replay already revealed one bug (#57450), there may be more.
I'll try running Fuzzlyn with this for a while before we merge it, and let me kick off some more CI legs.
/azp run runtime-coreclr jitstress, outerloop |
| Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-coreclr outerloop |
| Azure Pipelines successfully started running 1 pipeline(s). |
I ran Fuzzlyn for a while and didn't find anything that looked specific to this PR. The failures look like#58481. |
Thanks! |
Uh oh!
There was an error while loading.Please reload this page.
Just as the title says - remove the restriction that we do not propagate constants for small-typed trees. It is safe because VNadds the necessary truncating casts when evaluating the assignment.
Now, small-typed nodes themselves are a bit rare, but one scenario where they come up is in field-by-field copying and initialization of promoted structs, which is where practically all the diffs for this change come from.
The substantial code here is only in the third commit, the rest is miscellaneous cleanup.
Some nice diffs for this change:
win-x64,win-arm64,win-x86Small regressions here and there are due to slightly different register allocation and the fact that sometimes containing a constant is not profitable size-wise.
Because this is an impactful change, I think stress testing is warranted for this PR. SPMI replay already revealed one bug (#57450), there may be more.