Do not continue to merge back cold path if guard2 block has been removed #7297
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
|
@jdmpapin May I ask you to review this change? Thank you very much in advance! This comment gives a detailed example of the problem this PR fixes. @hzongaro fyi |
jdmpapin
reviewed
Mar 28, 2024
`changeBranchDestination` tries to remove unreachable blocks after the removal of the old edge from guard2 to cold2. Rarely, a previous transformation in this pass could have made guard2Block unreachable without removing it, in which case it might have been removed just now. Normally, if all relevant blocks are removed, later `moveBlockAfterDest` will not cause any issue since it will be just moving around all unreachable blocks. However, in an even more rare case, if some of the blocks are removed and some of them are not (eg, guard2Block and the previous block of guard2Block are removed but the next block of guard2Block is still valid), `moveBlockAfterDest` could end up joining an invalid/removed block with a valid block. Therefore, if guard2Block is no longer valid, it should not proceed. Fixes: eclipse-openj9/openj9#18873 Signed-off-by: Annabelle Huo <Annabelle.Huo@ibm.com>
a7ehuo
force-pushed
the
fix-VirtualGuardHeadMerger-1
branch
from
85bf10a
to
3f7c770
Compare
jdmpapin
approved these changes
Apr 3, 2024
|
The mac failure is #7181 Jenkins build all |
|
Everything has passed except for the known mac failure already mentioned |
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.
changeBranchDestinationtries to remove unreachable blocks after theremoval of the old edge from guard2 to cold2. Rarely, a previous
transformation in this pass could have made guard2Block unreachable
without removing it, in which case it might have been removed just now.
Normally, if all relevant blocks are removed, later
moveBlockAfterDestwill not cause any issue since it will be just moving around all
unreachable blocks. However, in an even more rare case, if some of the
blocks are removed and some of them are not (eg, guard2Block and the
previous block of guard2Block are removed but the next block of
guard2Block is still valid),
moveBlockAfterDestcould end up joiningan invalid/removed block with a valid block. Therefore, if guard2Block
is no longer valid, it should not proceed.
Fixes: eclipse-openj9/openj9#18873