Skip to content

Fix insertion of alignment after BBJ_CALLFINALLY/BBJ_ALWAYS#76988

Merged
BruceForstall merged 1 commit intodotnet:mainfrom
BruceForstall:Fix76910
Oct 13, 2022
Merged

Fix insertion of alignment after BBJ_CALLFINALLY/BBJ_ALWAYS#76988
BruceForstall merged 1 commit intodotnet:mainfrom
BruceForstall:Fix76910

Conversation

@BruceForstall
Copy link
Contributor

@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
@ghost ghost 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
Contributor Author

Failure is known infra issue.

@BruceForstall
Copy link
Contributor Author

@kunalspathak @dotnet/jit-contrib PTAL

Copy link
Contributor

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