-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
JIT: Add stress mode for skipping lowering conditional nodes to use CPU flags #103358
base: main
Are you sure you want to change the base?
Conversation
/azp run runtime-coreclr jitstress |
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Azure Pipelines successfully started running 1 pipeline(s). |
src/coreclr/jit/lower.cpp
Outdated
#ifdef DEBUG | ||
if (comp->compStressCompile(Compiler::STRESS_SKIP_COND_NODE_LOWERING, 50)) | ||
{ | ||
JITDUMP("JitStress: skip lowering attempt\n"); | ||
return false; | ||
} | ||
#endif // DEBUG |
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.
50% of methods seems a bit high given the frequency of conditional branches.. maybe something like 10% is sufficient?
Nit: the ifdef
is unnecessary, there is a release version of compStressCompile
that always returns false (but feel free to leave it if you prefer to make it more visible that this is debug-only).
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.
Sounds good, I'll update it.
This seems to have exposed some asserts in LSRA -- @jakobbotsch should we address these in a separate PR, or here so we don't block outerloop CI? |
I think we need to address it first. Maybe just open an issue with instructions about how to reproduce this (does it repro readily with DOTNET_JitStress enabled spmi replay on this PR?) |
Follow-up to #103319 (comment). @jakobbotsch PTAL, thank you!