Enable global constant propagation for small types #57726
Conversation
ghost
added
the
community-contribution
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
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 VN adds 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 check if a type can represent some value.
SingleAccretion
force-pushed
the
Enable-GCP-For-Small-Types
branch
from
808faf5 to
97f96dd
Compare
|
CC @dotnet/jit-contrib please review the community PR. |
There was a problem hiding this comment.
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! |
ghost
locked as resolved and limited conversation to collaborators
Just as the title says - remove the restriction that we do not propagate constants for small-typed trees. It is safe because VN adds 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.