Skip to content

add STRESS_PROMOTE_LESS_STRUCTS mode. #49084

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
Merged
sandreenko merged 5 commits into from
Mar 16, 2021
Merged

add STRESS_PROMOTE_LESS_STRUCTS mode. #49084

sandreenko merged 5 commits into from
Mar 16, 2021

Conversation

@sandreenko
Copy link
Contributor

@sandreenko sandreenko commented Mar 3, 2021

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

sandreenko commented Mar 3, 2021

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 3, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@sandreenko
Copy link
Contributor Author

sandreenko commented Mar 4, 2021

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 4, 2021

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

sandreenko commented Mar 4, 2021

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

azure-pipelines bot commented Mar 4, 2021

Azure Pipelines successfully started running 1 pipeline(s).

@AndyAyersMS
Copy link
Member

AndyAyersMS commented Mar 4, 2021

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

sandreenko commented Mar 5, 2021

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

@sandreenko
Copy link
Contributor Author

sandreenko commented Mar 5, 2021

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

AndyAyersMS commented Mar 9, 2021

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 suggested changes Mar 13, 2021
View reviewed changes
Copy link
Contributor

@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
Copy link
Contributor

@BruceForstall BruceForstall Mar 13, 2021

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

@sandreenko sandreenko Mar 15, 2021

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
Copy link
Contributor

@BruceForstall BruceForstall Mar 15, 2021

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

1 more reviewer

@BruceForstall BruceForstall BruceForstall approved these changes

Reviewers whose approvals may not affect merge requirements

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.

4 participants

@sandreenko @AndyAyersMS @BruceForstall @karelz