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.

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

Fix insertion of alignment after BBJ_CALLFINALLY/BBJ_ALWAYS #76988

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

BruceForstall
Copy link
Member

@BruceForstall BruceForstall commented Oct 13, 2022

The Runtime_76346 test exposed a case where, in the case of STRESS_EMITTER, we were inserting breakpoint instructions instead of NOPs for loop alignment when the alignment followed an unconditional branch. However, it wasn't considering the case of a BBJ_CALLFINALLY/BBJ_ALWAYS pair immediately followed by a loop head.

This pointed out that alignment instructions should never be inserted in that BBJ_ALWAYS block, since that block shouldn't contain any code. Inserting the alignment NOPs affected the reported range of the EH cloned finally region in the FEATURE_EH_CALLFINALLY_THUNKS case.

In these special cases, we simply give up on trying to align the loop.

Fixes #76910

No diffs

The Runtime_76346 test exposed a case where, in the case of STRESS_EMITTER,
we were inserting breakpoint instructions instead of NOPs for loop alignment
when the alignment followed an unconditional branch. However, it wasn't
considering the case of a BBJ_CALLFINALLY/BBJ_ALWAYS pair immediately
followed by a loop head.

This pointed out that alignment instructions should never be inserted in that
BBJ_ALWAYS block, since that block shouldn't contain any code. Inserting the
alignment NOPs affected the reported range of the EH cloned finally region
in the FEATURE_EH_CALLFINALLY_THUNKS case.

In these special cases, we simply give up on trying to align the loop.

Fixes dotnet#76910
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 13, 2022
@ghost ghost assigned BruceForstall Oct 13, 2022
@ghost
Copy link

ghost commented Oct 13, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

The Runtime_76346 test exposed a case where, in the case of STRESS_EMITTER, we were inserting breakpoint instructions instead of NOPs for loop alignment when the alignment followed an unconditional branch. However, it wasn't considering the case of a BBJ_CALLFINALLY/BBJ_ALWAYS pair immediately followed by a loop head.

This pointed out that alignment instructions should never be inserted in that BBJ_ALWAYS block, since that block shouldn't contain any code. Inserting the alignment NOPs affected the reported range of the EH cloned finally region in the FEATURE_EH_CALLFINALLY_THUNKS case.

In these special cases, we simply give up on trying to align the loop.

Fixes #76910

Author: BruceForstall
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@BruceForstall
Copy link
Member Author

Failure is known infra issue.

@BruceForstall
Copy link
Member Author

@kunalspathak @dotnet/jit-contrib PTAL

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its odd that this failure just showed up now.

@BruceForstall
Copy link
Member Author

Its odd that this failure just showed up now.

It's obviously quite rare in practice, as it requires (1) a try/finally immediately before a loop, (2) the 'finally' doesn't get optimized by finally cloning, (3) the loop header immediately follows, meaning we haven't done loop inversion.

@BruceForstall BruceForstall merged commit 9086686 into dotnet:main Oct 13, 2022
@BruceForstall BruceForstall deleted the Fix76910 branch October 13, 2022 19:14
@ghost ghost locked as resolved and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime_76346 fails in jitstress on arm64
2 participants