Allow allocationFence to be reenabled by Local Flush Elimination #16547
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flow Sensitive Escape Analysis will mark an allocationFence with omitSync set to true if it was needed by a single allocation that ended up being stack allocated. On a subsequent pass of Escape Analysis, Local Flush Elimination might decide an allocationFence is needed at the same spot for another object allocation. An assertion in TR_LocalFlushElimination::examineNode expects any existing allocationFence that it finds to have its omitSync set to false, which didn't account for the possibility that it might find one that had previously been disabled. Fixed the problem by removing the assertion and allowing the allocationFence to be reactivated for the second object allocation. Also added more information to some trace messages. Signed-off-by: Henry Zongaro <zongaro@ca.ibm.com>
|
Kevin @klangman and Vijay @vijaysun-omr, may I ask you to review this change? I know Kevin has some thoughts about fixes for additional problems that still exist in the interaction between escape analysis and flush elimination. This change is really an interim fix to deal with the fatal assertion failure. |
vijaysun-omr
approved these changes
Jan 12, 2023
|
Jenkins test sanity all jdk8,jdk11,jdk19 |
|
Looks good to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flow Sensitive Escape Analysis will mark an
allocationFencewithomitSyncset to true if it was needed by a single allocation that ended up being stack allocated.On a subsequent pass of Escape Analysis, Local Flush Elimination might decide an
allocationFenceis needed at the same spot for another object allocation. An assertion inTR_LocalFlushElimination::examineNodeexpects any existingallocationFencethat it finds to have itsomitSyncset to false, which didn't account for the possibility that it might find one that had previously been disabled.Fixed the problem by removing the assertion and allowing the
allocationFenceto be reactivated for the second object allocation.Also added more information to some trace messages.