Fix no struct promotion condition for long vars on 32bit platforms. #53067
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
sandreenko
added
the
area-CodeGen-coreclr
Contributor
Author
|
PTAL @dotnet/jit-contrib small cleaning change that came from another work item. |
kunalspathak
approved these changes
May 21, 2021
src/coreclr/jit/jitconfigvalues.h
Outdated
| CONFIG_INTEGER(JitNoMemoryBarriers, W("JitNoMemoryBarriers"), 0) // If 1, don't generate memory barriers | ||
| CONFIG_INTEGER(JitNoRegLoc, W("JitNoRegLoc"), 0) | ||
| CONFIG_INTEGER(JitNoStructPromotion, W("JitNoStructPromotion"), 0) // Disables struct promotion in Jit32 | ||
| CONFIG_INTEGER(JitNoStructPromotion, W("JitNoStructPromotion"), 0) // Disables struct promotion &1 - for all, &2 - for |
Contributor
There was a problem hiding this comment.
nit: What does &1 and &2 mean?
Contributor
Author
There was a problem hiding this comment.
I do not know how to describe it, it means
if ((JitNoStructPromotion & 1) != 0) fgNoStructPromotion = true
if ((JitNoStructPromotion & 2) != 0) fgNoStructParamPromotion= true
we use a similar "hash" approach for JitStress and JitMinOpts but they don't have such comments.
Contributor
There was a problem hiding this comment.
I see. Why not just pass JitNoStructPromotion=1, etc. and then check for if (JitNoStructPromotion == 1) fgNoStructPromotion = true and so forth?
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
The condition was
(fgNoStructPromotion && varDsc->lvIsParam)when it should befgNoStructPromotion || (fgNoStructParamPromotion && varDsc->lvIsParam)).While I am here also move
lvaPromoteLongVarstoDecomposeLongsand add a commentJitNoStructPromotion.There are tiny diffs in spmi tests on x86 from tests because we disable struct promotions in 2 cases:
runtime/src/coreclr/jit/importer.cpp
Lines 16474 to 16476 in b18be1e
and
runtime/src/coreclr/jit/importer.cpp
Lines 12130 to 12135 in b18be1e
and now, in these cases, we don't promote longs.