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

Make versioner skip loops that have become acyclic during the pass #6531

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

jdmpapin
Copy link
Contributor

The region structure is expected to be a loop. If it isn't, then versioner may misbehave, e.g. it may crash. And besides, there's no point in trying to version an acyclic region anyway.

HCR guard versioning removes edges from the CFG immediately (unlike versioning of regular conditionals/virtual guards, which leaves a trivially foldable conditional tree in place). The removal of particular edges can change loops into acyclic regions, so skip any region that is no longer a loop.

(OSR guard versioning also removes edges immediately, but the targets of those edges are not within any loop, so their removal can't change loops into acyclic regions.)

Fixes eclipse-openj9/openj9#15061

The region structure is expected to be a loop. If it isn't, then
versioner may misbehave, e.g. it may crash. And besides, there's no
point in trying to version an acyclic region anyway.

HCR guard versioning removes edges from the CFG immediately (unlike
versioning of regular conditionals/virtual guards, which leaves a
trivially foldable conditional tree in place). The removal of particular
edges can change loops into acyclic regions, so skip any region that is
no longer a loop.

(OSR guard versioning also removes edges immediately, but the targets of
those edges are not within any loop, so their removal can't change loops
into acyclic regions.)
@vijaysun-omr
Copy link
Contributor

Jenkins test sanity.functional,extended.functional all jdk17

@vijaysun-omr
Copy link
Contributor

Do we think we should consider not allowing HCR guard versioning to change trees immediately (thus creating this problem) in the future ?

@jdmpapin
Copy link
Contributor Author

Possibly. When this does happen, it's better to avoid versioning the "loop" (as done in this commit), but it should be an extremely rare case, so I don't expect any performance benefit

To understand how rare: Reproducing this reliably (since the repro attached to eclipse-openj9/openj9#15061 was intermittent), I had to create a truly bizarre configuration of two nested loops:

  • The outer loop had to contain an HCR guard that exits the loop on the passing edge.
  • The rest of the outer loop's body still had to be reachable (in the CFG) after deleting the HCR guard
  • Versioner's HCR guard search had to be able to tell that the rest of the outer loop's body (including the inner loop) was unreachable, so it had to be initially reachable only via the HCR guard's failing edge or via another conditional which gets versioned.
  • The inner loop had to have its back-edge dominated by another HCR guard's failing edge.

@jdmpapin
Copy link
Contributor Author

jdmpapin commented Jun 2, 2022

Jenkins build all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The JDK11 of J9 reports crashes in test cases
2 participants