Improve InterpreterEmulator's stateful bytecode iteration - #24504
Open
nbhuiyan wants to merge 10 commits into
Open
Improve InterpreterEmulator's stateful bytecode iteration#24504nbhuiyan wants to merge 10 commits into
nbhuiyan wants to merge 10 commits into
Conversation
- At each position, the merged Operand was computed but not stored back into the operand array. - The arrays were assumed to have length _numSlots, which is true for the locals' values, but not necessarily stack values.
Also assert when maintaining stack for return and ldc.
KnownObjOperand no longer accepts the null index, so starting now it's not possible for any Operand's getKnownObjectIndex() to indicate null. Instead, null will be represented by _nullOperand. Additionally, KnownObjOperand will now be created only when the compiler has successfully determined the class of the object.
The MethodHandle.asType folding for Invokers.checkGenericType in getReturnValue() was implemented with instantiation of KnownObjOperand directly. This commit fixes it to use the new helper. Signed-off-by: Nazim Bhuiyan <nubhuiyan@ibm.com>
This ensures that a block's predecessors are visited before the block itself when possible. Additionally, structuring the analysis as a loop over blocks will help in the later implementation of branch folding.
Guarded by env option TR_moreInterpreterEmulator.
This adds support for more, but not all bytecodes. Eventually, we will have all bytecodes supported in the InterpreterEmulator.
Guarded by env option TR_enableEarlyGuardedSFFF.
Member
Author
This file contains hidden or 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
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.
InterpreterEmulator can walk a callee's bytecodes while maintaining a model of the operand stack and locals, so the inliner can reason about values (which object a receiver is, whether a branch is constant, what a static final field holds) while it is still deciding what to inline. This stateful iteration mode was only ever used for MethodHandle thunk archetypes, LambdaForm-generated methods, and a very small set of other methods that fall outside of these two categories. Ordinary Java methods were always walked without state.
This PR lays the groundwork for using it more widely. It fixes correctness problems in the existing implementation, generalizes the operand model, and adds the value-based transformations that later work builds on. It does not change any default behaviour. This is all work done by Devin Papineau (@jdmpapin), with the exception of one commit, rebased on latest OpenJ9 main branch with the original commits formatted according the clang-format specifications. This PR introduces some "todo" comments inline left by Devin, and I left them in that state deliberately. These "todo"s are all cleaned up and addressed in the PR that will follow this one. This PR is the first part of a series of PRs that will improve InterpreterEmulator capabilities, and address performance issues seen in DirectMethodHandle-based Reflection implementation in JDK21+ (JEP 416).
It will make a lot more sense to review the changes commit-by-commit. I have included a high-level summary of the changes below.
Changes:
Correctness and cleanup:
mergeOperandArray()computed the merged operand for each slot and then discarded it, so merging at control flow joins silently did nothing. It also assumed both arrays were the length of the locals array, which is not true for the operand stack.dumpStack(), and extract the_iteratorWithStateassertion into a helper.Operand model:
NullOperandso null is represented directly, and establish the invariant that no operand's known object index ever denotes null. AllKnownObjOperandconstruction now goes through a singleknownObjOperand()helper that handles the unknown and null cases and looks up the class.MethodHandle.asTypefolding call sites added in 81da4ec to the new helper.Iteration and transformations:
shouldIterateWithState(), which opts into stateful iteration only when there is something to gain (a known object argument, or a foldable static final in the callee), and declines for AOT compilationsSome additional notes:
Everything here is off by default and gated behind two options,
TR_moreInterpreterEmulatorandTR_enableEarlyGuardedSFFF. With neither set, the compiler follows the same paths it does today.Note that the two options are not usable yet on their own in general scenarios. Stateful iteration still fails on some common opcodes in a way that rejects the callees outright (instead of, for example, falling back to stateless iteration). Furthermore, early folding results in unrestricted fear point placement, which then causes an assertion failure during
HCRGuardAnalysis. Both are addressed in the follow-up PR, and neither is reachable without setting the options.As reviewers, please let me know if I should address the two issues above in this PR and make this an even larger piece of work to review. I chose to deliver these changes on their own mainly to make it easier to review.