-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Do not over-align code when JitAlignLoops=0 is set #107340
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
if (emitComp->opts.OptimizationEnabled() && | ||
(!emitComp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT) || comp->IsTargetAbi(CORINFO_NATIVEAOT_ABI)) && | ||
(emitTotalHotCodeSize > 16) && emitComp->fgHasLoops) | ||
if (codeGen->ShouldAlignLoops() && (emitTotalHotCodeSize > 16) && emitComp->fgHasLoops) |
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.
Should we also set JitAlignLoops
to false
if we are optimizing for size and hence, here we will also adjust the method alignment to 8 bytes instead of 32 bytes?
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.
I’d say yes. I wanted to unblock my testing first which is why I submitted this separately.
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.
LGTM
Simplify the condition for alignment of methods with loop. The check for optimizations, R2R and NativeAOT already guards the
SetAlignLoops
code path, so there's no need to repeat it. Additionally, explicitly disabling loop alignment withJitAlignLoops=0
should disable the extra method alignment.