-
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
add STRESS_PROMOTE_LESS_STRUCTS mode. #49084
Conversation
/azp run runtime-coreclr jitstress |
Azure Pipelines successfully started running 1 pipeline(s). |
22a8e8c
to
21b7af7
Compare
/azp run runtime-coreclr jitstress |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-coreclr jitstress |
Azure Pipelines successfully started running 1 pipeline(s). |
Feel free to read, steal from, or ignore some related old changes of mine main...AndyAyersMS:StructPromotionStress |
@AndyAyersMS I think it is better to merge such changes separately. |
PTAL @dotnet/jit-contrib this PR adds a new stress mode (and disables tests that are failing because of it, opened #49189). In this stress mode, we randomly reject some struct promotions in Right now |
You could add the implicit byref stress easily enough, it does not require refactoring. |
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.
One grammar nit: this should be named STRESS_PROMOTE_FEWER_STRUCTS
(and other "promote less structs" should be replaced with "promote fewer structs")
src/coreclr/jit/compiler.cpp
Outdated
// true if this local should not be promoted. | ||
// | ||
// Notes: | ||
// Reject ~50% of the potential promotions if STRESS_PROMOTE_LESS_STRUCTS is active. |
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.
So, the stress mode will kick in for 50% of functions, and 50% of the structs in those methods that otherwise would have been promoted will not be.
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.
Yes, this is an exact description.
src/coreclr/jit/compiler.h
Outdated
@@ -9242,6 +9242,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX | |||
STRESS_MODE(SWITCH_CMP_BR_EXPANSION) \ | |||
STRESS_MODE(GENERIC_VARN) \ | |||
STRESS_MODE(PROFILER_CALLBACKS) /* Will generate profiler hooks for ELT callbacks */ \ | |||
STRESS_MODE(PROMOTE_LESS_STRUCTS) /* Don't promote some structs that can be promoted */ \ |
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.
You need to change this one too
21233b9
to
5095af2
Compare
Right now we don't have profitability heuristics for promotion, so everything that can be promoted is promoted.
It makes most structs with the same type to be always promoted and we see only their fields.
This mode rejects promotion for some lclVars so we can see more copies like
LCL_VAR structType1 V01, promoted = LCL_VAR structType1, not promoted
that are harder for codegen.