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

JIT: avoid cloning mid-entry loops with multiple non-loop entry preds #70959

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

AndyAyersMS
Copy link
Member

If a mid-entry loop has several non-loop preds, some of them may be edges from
enclosing loop constructs that were not recognized as loops on their own.

Avoid cloning such loops. We won't do proper invariance anlysis as we are not
properly recognizing the extent of the loop as is, and we won't get the flow
connnected up properly post cloning (so that the fast loop is proper loop where
the entry is dominated by the head).

This is a workaround for a fairly rare case. Such loops are never iterable and
so we will only try cloning when these loops have an invariant type test.

Ideally we would extend loop canonicalization to cover this case, essentially
ensuring that all recognized loops have a preheader -- that is, that the entry
has just one non-loop predecessor. Currently we do this only for top entry
loops. But doing that with our current setup looked more complex and we don't
expect to see many of these cases.

Closes #70802.

If a mid-entry loop has several non-loop preds, some of them may be edges from
enclosing loop constructs that were not recognized as loops on their own.

Avoid cloning such loops. We won't do proper invariance anlysis as we are not
properly recognizing the extent of the loop as is, and we won't get the flow
connnected up properly post cloning (so that the fast loop is proper loop where
the entry is dominated by the head).

This is a workaround for a fairly rare case. Such loops are never iterable and
so we will only try cloning when these loops have an invariant type test.

Ideally we would extend loop canonicalization to cover this case, essentially
ensuring that all recognized loops have a preheader -- that is, that the entry
has just one non-loop predecessor. Currently we  do this only for top entry
loops. But doing that with our current setup looked more complex and we don't
expect to see many of these cases.

Closes dotnet#70802.
@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 Jun 19, 2022
@ghost ghost assigned AndyAyersMS Jun 19, 2022
@ghost
Copy link

ghost commented Jun 19, 2022

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

Issue Details

If a mid-entry loop has several non-loop preds, some of them may be edges from
enclosing loop constructs that were not recognized as loops on their own.

Avoid cloning such loops. We won't do proper invariance anlysis as we are not
properly recognizing the extent of the loop as is, and we won't get the flow
connnected up properly post cloning (so that the fast loop is proper loop where
the entry is dominated by the head).

This is a workaround for a fairly rare case. Such loops are never iterable and
so we will only try cloning when these loops have an invariant type test.

Ideally we would extend loop canonicalization to cover this case, essentially
ensuring that all recognized loops have a preheader -- that is, that the entry
has just one non-loop predecessor. Currently we do this only for top entry
loops. But doing that with our current setup looked more complex and we don't
expect to see many of these cases.

Closes #70802.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@BruceForstall PTAL
cc @dotnet/jit-contrib

No diffs.

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.

LGTM. Looks like another case where always creating pre-headers would fix the problem.

@AndyAyersMS
Copy link
Member Author

LGTM. Looks like another case where always creating pre-headers would fix the problem.

It would, provided we migrated the non-loop preds to the preheader.

@AndyAyersMS AndyAyersMS merged commit 925e26e into dotnet:main Jun 21, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 22, 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
2 participants