[0.42.0] Prevent commoning of l2a operations in localCSE #192
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
An l2a operation might be used to convert the result of a compressed reference sequence to an address, but it is also used in other circumstances - for instance, if an Int64 value holds the address of a j9class or a j9method. The l2aEvaluators assume that l2a is used for compressed references or accessing arraylets. If the node is not accessing an arraylet and compressed references are enabled, the evaluator will mark the source register as containing a collected reference under the assumption that it's working with a compressed reference. However, if the l2a is being used for a j9class, j9method, etc., the register that holds it should not be marked as a collected reference. As an interim fix, this change will prevent commoning of l2a operations. The various OMR::<CPU>::MemoryReference::populateMemoryReference code generation methods will skip evaluation of an l2a that has a reference count of one, so that prevents the register containing the operand of the l2a from being marked as a collected reference. In the longer term, OMR issue 6508 proposes introducing a new opcode - l2gcref - that would be used to mark explicitly the result of a conversion from long to an address as holding a collected reference. Once implemented, the change implemented by this commit can be reverted.
|
@pshipton, may I ask you to merge this change? |
vijaysun-omr
approved these changes
Jan 8, 2024
pshipton
approved these changes
Jan 8, 2024
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.
An
l2aoperation might be used to convert the result of a compressed reference sequence to an address, but it is also used in other circumstances - for instance, if anInt64value holds the address of aj9classor aj9method. Thel2aEvaluators[1,2,3,4] assume thatl2ais used for compressed references or accessing arraylets. If the node is not accessing an arraylet and compressed references are enabled, the evaluator will mark the source register as containing a collected reference under the assumption that it's working with a compressed reference. However, if thel2ais being used for aj9class,j9method, etc., the register that holds it should not be marked as a collected reference.As an interim fix, this change will prevent commoning of
l2aoperations. The variousOMR::<CPU>::MemoryReference::populateMemoryReferencecode generation methods[5,6,7,8] will usually skip evaluation of anl2athat has a reference count of one, so that prevents the register containing the operand of thel2afrom being marked as a collected reference.In the longer term, OMR issue #6508 proposes introducing a new opcode -
l2gcref- that would be used to mark explicitly the result of a conversion from long to an address as holding a collected reference. Once implemented, the change implemented by this commit can be reverted.This will work around the problem reported in OpenJ9 issue eclipse-openj9/openj9#18098 as affecting
SharedClasses.SCM01.MultiThread_1. Other test failures could have other causes that will need further investigation.This delivers the contents of OMR pull request eclipse-omr/omr#7217 to the 0.42.0-release branch.
[1] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/aarch64/codegen/UnaryEvaluator.cpp#L640-L641
[2] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/p/codegen/UnaryEvaluator.cpp#L573-L574
[3] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/x/codegen/UnaryEvaluator.cpp#L381-L382
[4] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/z/codegen/UnaryEvaluator.cpp#L251-L252
[5] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/aarch64/codegen/OMRMemoryReference.cpp#L587-L612
[6] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/p/codegen/OMRMemoryReference.cpp#L587-L597
[7] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/x/codegen/OMRMemoryReference.cpp#L521-L533
[8] https://github.com/eclipse/omr/blob/3a4787401fffbdbb8088021d3553915ea3ff8edf/compiler/z/codegen/OMRMemoryReference.cpp#L1801-L1809