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

Refactor SFFF for InterpreterEmulator and record/replay InterpreterEmulator constants #19087

Merged
merged 5 commits into from
Mar 25, 2024

Conversation

jdmpapin
Copy link
Contributor

@jdmpapin jdmpapin commented Mar 6, 2024

This series consists of some initial preparations for expanded static final field folding (SFFF) in InterpreterEmulator, primarily in the two ways mentioned in the title. Brief summary of each commit:

  • Make KnownObjectTable::addStableArray() safe to call on the server: needed for the following refactoring
  • Refactor static final field folding for reuse in InterpreterEmulator: exposes SFFF logic that will facilitate later guarded/speculative folding in inliner
  • Declare isFearPointPlacementUnrestricted(): names a concept that will be needed later for early guarded folding
  • Delete unused flags in the IL generator held over from multitenancy: minor cleanup prior to record/replay
  • Record/replay InterpreterEmulator constants: ensures consistency between inliner and IL, which will become more important later with expanded folding, and which will also inform the placement of fear points for guarded folding

This PR depends on eclipse/omr#7282

@jdmpapin jdmpapin requested a review from dsouzai as a code owner March 6, 2024 20:23
@jdmpapin jdmpapin requested review from 0xdaryl and vijaysun-omr and removed request for dsouzai March 6, 2024 20:31
@jdmpapin
Copy link
Contributor Author

jdmpapin commented Mar 6, 2024

@0xdaryl and/or @vijaysun-omr, please review

@jdmpapin
Copy link
Contributor Author

jdmpapin commented Mar 12, 2024

Converted to draft because I need to also record/replay constants from array element loads

edit: Never mind, InterpreterEmulator doesn't know yet how to fold array elements. There is some logic for that in my copy, but it's part of a later WIP change that implements support for additional opcodes

@jdmpapin jdmpapin marked this pull request as ready for review March 13, 2024 17:37
Only the assertion needs to be adjusted. To avoid getPointer() and
getObjectClass(), it now calls getObjectClassFromKnownObjectIndex()
instead. Because that API can fail to acquire VM access, the assertion
is relaxed to pass when the class is not available.
This positions InterpreterEmulator to be modified later for speculative
folding.

- canFoldStaticFinalField() now has a variant that doesn't need a node.

- knownObjectFromFinalStatic() is deleted to consolidate logic.

- Callers now use a combination of canFoldStaticFinalField() and
  staticFinalFieldValue(), which is not limited to known objects, and
  which can handle fields that require OSR assumptions to fold.

There are some small behavioural changes:

- In AOT, canFoldStaticFinalField() now results in TR_no for everything
  but String.COMPACT_STRINGS. Previously, it would give the same result
  as in non-AOT, and folding would be attempted only to fail.

- The aggressive VarHandle folding implemented in 4aa88c0 is now
  applied when the declared type of the field is VarHandle rather than
  whenever its value happens to be an instance of VarHandle. It's also
  restricted to cases in which the declaring class is in the JCL or has
  version >=53 to ensure that the field won't be modified by putstatic.
  Classes that declare fields of type VarHandle should have version >=53
  anyway, since version 53 corresponds to Java 9, which is the version
  in which VarHandle was first introduced. Finally, this VarHandle
  folding can now be disabled by an environment variable.
Initially the result is always false. It will be updated later to
sometimes return true, but in the meantime there is already a name for
the concept.
Inlining uses constant folding to help determine call targets, so it's
important for the corresponding IL (if any is generated) to be
consistent with what the inliner saw.

InterpreterEmulator now remembers constants for each TR_CallTarget that
must be folded consistently in the IL, and IL generation consults these
to repeat the same constant folding as needed.

This repeated folding is important whenever a value might be allowed to
be folded despite the possibility of a later change. It also allows
constants to be speculative by specifying the assumptions that are
necessary in order for the folding to be correct, and by informing the
IL generator of the locations where such assumptions have been made.
However, InterpreterEmulator does not yet do speculative folding.

This commit also removes a bunch of unused code, mostly related to class
lookahead, from TR_J9ByteCodeIlGenerator::loadStatic().
@jdmpapin
Copy link
Contributor Author

Rebased to fix the MINOR_NUMBER conflict

@0xdaryl
Copy link
Contributor

0xdaryl commented Mar 25, 2024

Jenkins test sanity all jdk21

@0xdaryl 0xdaryl self-assigned this Mar 25, 2024
@0xdaryl 0xdaryl merged commit f4a6293 into eclipse-openj9:master Mar 25, 2024
13 checks passed
@knn-k
Copy link
Contributor

knn-k commented Mar 26, 2024

I opened PR #19222 for fixing a build break on macOS.

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.

None yet

3 participants