Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add STRESS_PROMOTE_LESS_STRUCTS mode. #49084

Merged
merged 5 commits into from
Mar 16, 2021
Merged

add STRESS_PROMOTE_LESS_STRUCTS mode. #49084

merged 5 commits into from
Mar 16, 2021

Conversation

sandreenko
Copy link
Contributor

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.

@sandreenko sandreenko added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 3, 2021
@sandreenko
Copy link
Contributor Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sandreenko
Copy link
Contributor Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sandreenko sandreenko marked this pull request as draft March 4, 2021 07:55
@sandreenko
Copy link
Contributor Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AndyAyersMS
Copy link
Member

Feel free to read, steal from, or ignore some related old changes of mine main...AndyAyersMS:StructPromotionStress

@sandreenko sandreenko marked this pull request as ready for review March 5, 2021 03:30
@sandreenko
Copy link
Contributor Author

@AndyAyersMS I think it is better to merge such changes separately.

@sandreenko
Copy link
Contributor Author

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 shouldPromote to create more "ASG(promoted, unpromoted)" cases.

Right now shouldPromote is very naive because it does not have access to use counters, with this stress mode we prepare Jit for a better heuristic.

@AndyAyersMS
Copy link
Member

I think it is better to merge such changes separately.

You could add the implicit byref stress easily enough, it does not require refactoring.

BruceForstall
BruceForstall requested changes Mar 13, 2021
View reviewed changes
Copy link
Member

@BruceForstall BruceForstall left a 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.
Copy link
Member

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.

Copy link
Contributor Author

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.

BruceForstall
BruceForstall reviewed Mar 15, 2021
View reviewed changes
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 */ \
Copy link
Member

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

@sandreenko sandreenko merged commit e614e17 into dotnet:main Mar 16, 2021
@sandreenko sandreenko deleted the addStressMode branch March 16, 2021 21:33
@sandreenko sandreenko mentioned this pull request Mar 19, 2021
Closed
@sandreenko sandreenko mentioned this pull request Apr 9, 2021
Closed
10 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Apr 15, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BruceForstall BruceForstall BruceForstall approved these changes

Assignees
No one assigned
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sandreenko @AndyAyersMS @BruceForstall @karelz