Inline SegmentViewVarHandle operations in JDK21+ #20146

nbhuiyan · 2024-09-10T19:46:20Z

This changeset enables inlining of SegmentViewVarHandle operations
that have been introduced in JDK21 through the following:

Enable static final field folding for field types involved in such
VarHandle operations: ValueLayout, ValueLayouts, MemorySegment
Add recognized method info for
ValueLayouts$AbstractValueLayout.accessHandle and MemorySegment methods
Add VM API getLayoutVarHandle to obtain the layout VH object info
Add getReturnValue handler in InterpreterEmulator to evaluate the
result of accessHandle()
During ECS, set NeedsPeekingHeuristics to true for the caller of
MemorySegment methods so that the final field loads in the caller
that are relevant for obtaining the layout VH can be folded.
Finally, ECS now sets MemorySegment methods to iterate with
state in the InterpreterEmulator.

In addition to the above, this PR also introduces changes to how NeedsPeekingHeuristics is used, as well as added capability to look up resolved interface methods that are default methods defined within the interface class.

nbhuiyan · 2024-09-10T19:47:00Z

@jdmpapin I'd appreciate your review for these changes.

0xdaryl · 2024-09-10T20:09:01Z

@0xdaryl : tagging myself

runtime/compiler/env/VMJ9.cpp

runtime/compiler/env/j9method.cpp

runtime/compiler/optimizer/J9EstimateCodeSize.cpp

runtime/compiler/optimizer/J9TransformUtil.cpp

jdmpapin · 2024-09-17T21:02:05Z

Skipping windows to avoid stressing the single available build machine. These changes are all platform-agnostic. JDK17 is there to build and test without JAVA_SPEC_VERSION >= 21

Jenkins test sanity.functional,sanity.openjdk alinux64,plinux,xlinux,zlinux,aix,xmac,amac jdk17,jdk21,jdk23

jdmpapin · 2024-09-18T19:08:55Z

Most of the failures are known issues:

but the JDK21 sanity.openjdk failures were assertion failures in InterpreterEmulator:

Assertion failed at /home/jenkins/workspace/Build_JDK21_x86-64_linux_Personal/openj9/runtime/compiler/optimizer/InterpreterEmulator.cpp:912: false
VMState: 0x000501ff
	Can't maintain stack for unresolved invokehandle

which is here:

openj9/runtime/compiler/optimizer/InterpreterEmulator.cpp

Line 912 in 5bf2f17

TR_ASSERT_FATAL(false, "Can't maintain stack for unresolved invokehandle");

As part of some in-progress work on InterpreterEmulator, I have a change that deals with this:

--- a/runtime/compiler/optimizer/InterpreterEmulator.cpp
+++ b/runtime/compiler/optimizer/InterpreterEmulator.cpp
@@ -922,7 +1170,7 @@ InterpreterEmulator::maintainStackForCall()
    {
    assertHasState();
 
-   int32_t numOfArgs = 0;
+   int32_t numOfArgs = -1;
    TR::DataType returnType = TR::NoType;
    Operand* result = NULL;
 
@@ -960,18 +1208,47 @@ InterpreterEmulator::maintainStackForCall()
          case J9BCinvokestatic:
             isStatic = true;
             break;
+
          case J9BCinvokedynamic:
-         case J9BCinvokehandle:
-            TR_ASSERT_FATAL(false, "Can't maintain stack for unresolved invokehandle");
+            {
+            // Find the signature, which corresponds to the arguments on the stack.
+            // cpIndex is really the invokedynamic call site index
+            J9ROMClass *romClass = TR::Compiler->cls.romClassOf(method()->classOfMethod());
+            J9SRP *namesAndSigs = (J9SRP*)J9ROMCLASS_CALLSITEDATA(romClass);
+            J9ROMNameAndSignature *nameAndSig = NNSRP_GET(namesAndSigs[cpIndex], J9ROMNameAndSignature*);
+            J9UTF8 *sig = J9ROMNAMEANDSIGNATURE_SIGNATURE(nameAndSig);
+
+            // Parse the signature to determine the number of arguments and
+            // whether or not there is a return value.
+            U_8 sigTypes[256]; // signatures are limited to 255 params + 1 return type
+            UDATA numParams = 0;
+            UDATA numParamSlots = 0;
+            jitParseSignature(sig, sigTypes, &numParams, &numParamSlots);
+            numOfArgs = numParams;
+
+            // returnType is only used to distinguish void return (TR::NoType)
+            // from non-void return (any other value), so it's not necessary to
+            // get the correct non-void type here.
+            if (sigTypes[numParams] == J9_NATIVE_TYPE_VOID)
+               returnType = TR::NoType;
+            else
+               returnType = TR::Int32;
+
             break;
+            }
 
          default:
             break;
          }
-      TR::Method * calleeMethod = comp()->fej9()->createMethod(trMemory(), _calltarget->_calleeMethod->containingClass(), cpIndex);
-      numOfArgs = calleeMethod->numberOfExplicitParameters() + (isStatic ? 0 : 1);
-      returnType = calleeMethod->returnType();
+
+      if (numOfArgs < 0)
+         {
+         TR::Method * calleeMethod = comp()->fej9()->createMethod(trMemory(), _calltarget->_calleeMethod->containingClass(), cpIndex);
+         numOfArgs = calleeMethod->numberOfExplicitParameters() + (isStatic ? 0 : 1);
+         returnType = calleeMethod->returnType();
+         }
       }
+
    maintainStackForCall(result, numOfArgs, returnType);
    }

runtime/compiler/env/VMJ9.cpp

jdmpapin · 2024-09-19T14:20:07Z

The changes in InterpreterEmulator::maintainStackForCall() are mine, so I think we should have somebody else review those. @0xdaryl, would you mind reviewing them?

0xdaryl

I reviewed and approve the changes to InterpreterEmulator.cpp.

jdmpapin · 2024-09-19T14:48:35Z

Jenkins test sanity.functional,sanity.openjdk alinux64,plinux,xlinux,zlinux,aix,xmac,amac jdk17,jdk21,jdk23

jdmpapin · 2024-09-20T18:09:08Z

The JDK21 sanity.openjdk failures are all #20200. The z Linux sanity.functional failures are #20150, plus #19027 and #14706 in JDK21 only

Three other jobs ran into what I think are infrastructure problems. I'll restart them, but first please rebase to fix the conflict

In TR_ResolvedJ9Method::getResolvedInterfaceMethod, if the the class of the interface method is an interface class, then the resulting resolved method was just discarded, as the interface class was not expected to contain any interface method implementation prior to default interface methods being introduced in Java 8. This change removes the condition ensuring that the interface methods are not inside an interface class, enabling this function to obtain resolved interface methods that are default implementations. Signed-off-by: Nazim Bhuiyan <[email protected]>

NPH was being asked if peeking is necessary right after init, prior to any of the bytecode iteration was taking place, following which NPH.doPeeking() could return true. This meant that NPH was not having any impact on whether any peeking took place, whether the heuristics determined it being useful or not. This commit reorders the processBytecodeAndGenerateCFG phase of ECS to take place prior to the step checking whether peeking is necessary, so that if and when peeking is necessary, NPH can utilized to trigger ILGen with the new function setNeedsPeekingToTrue(). Previously, _needsPeeking was set to true if there was a param load within distance, which is now disabled so as not to modify the ECS behaviour where NPH was not having any impact on peeking. Only by explicitly using NPH.setNeedsPeekingToTrue() can peeking be triggered. Signed-off-by: Nazim Bhuiyan <[email protected]>

This changeset enables inlining of SegmentViewVarHandle operations that have been introduced in JDK21 through the following: * Add recognized method info for ValueLayouts$AbstractValueLayout.accessHandle and MemorySegment methods. * Add VM API getLayoutVarHandle to obtain the layout VH object info * Add getReturnValue handler in InterpreterEmulator to evaluate the result of accessHandle() * During ECS, set NeedsPeekingHeuristics to true for the caller of MemorySegment methods so that the final field loads in the caller that are relevant for obtaining the layout VH can be folded. * ECS now sets MemorySegment methods to iterate with state in the InterpreterEmulator. * In InterpreterEmulator:: maintainStackForCall(), obtain the arg count for invokedynamic calls by parsing the signature obtained from the call site Signed-off-by: Nazim Bhuiyan <[email protected]> Co-authored-by: Devin Papineau <[email protected]>

This commit adds JITServer support for the FE helper VM_getLayoutVarHandle. Signed-off-by: Nazim Bhuiyan <[email protected]>

nbhuiyan · 2024-09-20T19:39:07Z

Looks like my ECA expired. I signed it again but it might take some time until that updates.

jdmpapin · 2024-09-20T19:41:25Z

As far as I can tell the ECA check failure is spurious. I put your email address (copied from a commit) into the "ECA validation tool" on the check failure details page, and it said that you had a valid ECA

jdmpapin · 2024-09-20T19:46:21Z

Jenkins test sanity.functional,sanity.openjdk alinux64 jdk21

jdmpapin · 2024-09-20T19:51:37Z

Jenkins test sanity.functional xmac jdk17

jdmpapin · 2024-09-20T20:08:01Z

Jenkins test sanity.openjdk aix jdk23

jdmpapin · 2024-09-23T14:09:54Z

The JDK21 AArch64 Linux sanity.openjdk failure is #20200, like the other JDK21 sanity.openjdk failures were, so all failures have been known issues

The ECA check has also passed now

nbhuiyan requested a review from dsouzai as a code owner September 10, 2024 19:46

0xdaryl added the comp:jit label Sep 10, 2024

jdmpapin self-assigned this Sep 11, 2024

jdmpapin reviewed Sep 12, 2024

View reviewed changes

nbhuiyan force-pushed the mem-segment-vh-0 branch 2 times, most recently from 5c8b2b9 to 27c988f Compare September 16, 2024 20:04

jdmpapin approved these changes Sep 17, 2024

View reviewed changes

jdmpapin reviewed Sep 18, 2024

View reviewed changes

runtime/compiler/env/VMJ9.cpp Outdated Show resolved Hide resolved

nbhuiyan force-pushed the mem-segment-vh-0 branch from 27c988f to 54d2766 Compare September 19, 2024 00:53

0xdaryl approved these changes Sep 19, 2024

View reviewed changes

nbhuiyan and others added 3 commits September 20, 2024 15:28

nbhuiyan force-pushed the mem-segment-vh-0 branch from 54d2766 to 63941ca Compare September 20, 2024 19:31

Override SegmentViewVarHandle-related helper for JITServer

c41c05f

This commit adds JITServer support for the FE helper VM_getLayoutVarHandle. Signed-off-by: Nazim Bhuiyan <[email protected]>

nbhuiyan force-pushed the mem-segment-vh-0 branch from 63941ca to c41c05f Compare September 20, 2024 19:35

jdmpapin merged commit 25f0b72 into eclipse-openj9:master Sep 23, 2024
9 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inline SegmentViewVarHandle operations in JDK21+ #20146

Inline SegmentViewVarHandle operations in JDK21+ #20146

nbhuiyan commented Sep 10, 2024 •

edited

Loading

nbhuiyan commented Sep 10, 2024

0xdaryl commented Sep 10, 2024

jdmpapin commented Sep 17, 2024

jdmpapin commented Sep 18, 2024

jdmpapin commented Sep 19, 2024

0xdaryl left a comment

jdmpapin commented Sep 19, 2024

jdmpapin commented Sep 20, 2024

nbhuiyan commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 23, 2024

Inline SegmentViewVarHandle operations in JDK21+ #20146

Inline SegmentViewVarHandle operations in JDK21+ #20146

Conversation

nbhuiyan commented Sep 10, 2024 • edited Loading

nbhuiyan commented Sep 10, 2024

0xdaryl commented Sep 10, 2024

jdmpapin commented Sep 17, 2024

jdmpapin commented Sep 18, 2024

jdmpapin commented Sep 19, 2024

0xdaryl left a comment

Choose a reason for hiding this comment

jdmpapin commented Sep 19, 2024

jdmpapin commented Sep 20, 2024

nbhuiyan commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 20, 2024

jdmpapin commented Sep 23, 2024

nbhuiyan commented Sep 10, 2024 •

edited

Loading