Improve x86 inline checkcast/instanceof sequences for array cast classes by 0xdaryl · Pull Request #23383 · eclipse-openj9/openj9

0xdaryl · 2026-02-20T19:31:28Z

This PR makes two contributions to JIT generated code on x86-64:

Inline instanceof checks when cast class is [java/lang/Object

Extend the current x86 checkcast optimization when the cast class is a
java/lang/Object array to handle instanceof and isAssignableFrom() cases.

Inline checkcast/instanceof checks when the cast class is a known array on x86

Essentially inlines the checkcast/instanceof/isAssignableFrom() sequence when the
cast class is an array that the VM implements [1], but specializes it for when the
cast class is an array known at compile-time. It performs:

An exact equality check
A check for a match in the cast class cache
An arity check followed by a subclass check on the array leaf components when the
arities are the same
Updates the cast class cache on success/failure in the same manner that the VM
implementation does

Any “unusual” cases are punted to the VM (e.g., mismatched arities, when the leaf
class is an interface), as well as to throw the CastClassException if required.

The opt can be disabled by setting TR_DisableInlineArrayExactCastClass.

[1]

openj9/runtime/oti/VMHelpers.hpp

Line 589 in 9698be0

    
           inlineCheckCast(J9Class *instanceClass, J9Class *castClass, bool updateCache = true)

Extend the current x86 checkcast optimization when the cast class is a java/lang/Object array to handle instanceof and isAssignableFrom() cases. Signed-off-by: Daryl Maier <maier@ca.ibm.com>

…ay on x86 Essentially inlines the checkcast/instanceof/isAssignableFrom() sequence when the cast class is an array that the VM implements [1], but specializes it for when the cast class is an array known at compile-time. It performs: * An exact equality check * A check for a match in the cast class cache * An arity check followed by a subclass check on the array leaf components when the arities are the same * Updates the cast class cache on success/failure in the same manner that the VM implementation does Any “unusual” cases are punted to the VM (e.g., mismatched arities, when the leaf class is an interface), as well as to throw the CastClassException if required. The opt can be disabled by setting `TR_DisableInlineArrayExactCastClass`. [1] https://github.com/eclipse-openj9/openj9/blob/9698be0c19fdc436c2fd6a0588dbafcfc0dd76fa/runtime/oti/VMHelpers.hpp#L589 Signed-off-by: Daryl Maier <maier@ca.ibm.com>

0xdaryl · 2026-02-20T19:37:09Z

@a7ehuo : may I ask you to review this please? @vijaysun-omr FYI

a7ehuo

Here is my review on the first commit. I'll review the second commit next Monday.

a7ehuo · 2026-02-20T21:15:43Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

-         logprintf(comp->getOption(TR_TraceCG), comp->log(), "Inline checkcast for [jlO : node=%p", node);
+         if (reportInlineObjectArrayCheck)
+            {
+            OMR::CStdIOStreamLogger::Stdout->printf("XXXXX Inline checkcast for [jlO : isCheckCast=%d (icall=%d) : %s\n", isCheckCast, (node->getOpCodeValue() == TR::icall), comp->signature());


What is XXXXX in the trace for? Should it be removed?

Is OMR::CStdIOStreamLogger logging into verbose vlog, or onto the console, or into compilation log? The reason I'm asking is that if this information should be logged into the compilation log as well at line 4164 if comp->getOption(TR_TraceCG) is true.

The message Inline checkcast for ... might be confusing since it could be instanceof or isAssignableFrom cases.

a7ehuo · 2026-02-20T21:41:16Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+   static char *disableInlineObjectArrayCheck = feGetEnv("TR_DisableInlineObjectArrayCheck");

-   if (!disableInlineObjectArrayCheckCast && isCheckCast && clazz && TR::Compiler->cls.isClassArray(comp, clazz))
+   bool isRelocatableCompile = comp->compileRelocatableCode() || comp->isOutOfProcessCompilation();


isRelocatableCompile is not used in this commit. It should be removed in this commit. (I assumed likely it'd be used in the next commit)

a7ehuo · 2026-02-20T22:06:32Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

         generateMemImmInstruction(TR::InstOpCode::TEST4MemImm4, node,
            generateX86MemoryReference(romClassReg, offsetof(J9ROMClass, modifiers), cg), J9AccClassArray, cg);
-         generateLabelInstruction(TR::InstOpCode::JE4, node, outlinedCallLabel, cg);
+         generateLabelInstruction(TR::InstOpCode::JE4, node, outlinedHelperCallLabel, cg);


I wonder if this line could be updated as below to avoid going to the helper if it is not array for instanceof and isAssignableFrom cases since resultReg has already been initialized as zero.

generateLabelInstruction(TR::InstOpCode::JE4, node, isCheckCast ? outlinedHelperCallLabel : fallThruLabel, cg);

a7ehuo · 2026-02-20T22:07:33Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

         generateMemImmInstruction(TR::InstOpCode::TEST4MemImm4, node,
            generateX86MemoryReference(romClassReg, offsetof(J9ROMClass, modifiers), cg), J9AccClassInternalPrimitiveType, cg);
-         generateLabelInstruction(TR::InstOpCode::JNE4, node, outlinedCallLabel, cg);
+         generateLabelInstruction(TR::InstOpCode::JNE4, node, outlinedHelperCallLabel, cg);


I wonder if this line could be updated as below to avoid going to the helper if it is primitive array for instanceof and isAssignableFrom cases since resultReg has already been initialized as zero.

generateLabelInstruction(TR::InstOpCode::JNE4, node, isCheckCast ? outlinedHelperCallLabel : fallThruLabel, cg);

a7ehuo · 2026-02-20T22:34:48Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

         if (!objectNode->isNonNull())
            {


Would it be a problem for isAssignableFrom(Class<?> cls)? If cls is null, NullPointerException (not return false) should be thrown, but the current behaviour would return 0/false by jumping to fallThruLabel.

// If the object is NULL, no exception is thrown for a checkcast and a 0 // is returned for an instanceof. // if (!objectNode->isNonNull()) { generateRegRegInstruction(TR::InstOpCode::TESTRegReg(), node, objectReg, objectReg, cg); generateLabelInstruction(TR::InstOpCode::JE4, node, fallThruLabel, cg); }

vijaysun-omr · 2026-02-21T10:02:06Z

Is it possible to show the sequence from a JIT trace log after instruction selection (to help with review) ? Thanks

a7ehuo

Here is the second half of the review

a7ehuo · 2026-02-22T17:59:46Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+            OMR::CStdIOStreamLogger::Stdout->printf("YYYYY Found inlineArrayExactCastClass : isCheckCast=%d : %s\n", isCheckCast, comp->signature());
+            }
+
+         logprintf(comp->getOption(TR_TraceCG), comp->log(), "Inline instanceof/checkcast for const cast class array: node=%p", node);


Could the compilation log be updated to include isCheckCast value and mention isAssignableFrom case?

a7ehuo · 2026-02-22T18:17:42Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+         //
+         if (reportInlineArrayExactCastClass)
+            {
+            OMR::CStdIOStreamLogger::Stdout->printf("YYYYY Found inlineArrayExactCastClass : isCheckCast=%d : %s\n", isCheckCast, comp->signature());


What does YYYYY refer to? Should it be removed or updated to something more meaningful?

a7ehuo · 2026-02-22T18:27:36Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+         if (!objectNode->isNonNull())
+            {
+            generateRegRegInstruction(TR::InstOpCode::TESTRegReg(), node, objectReg, objectReg, cg);
+
+            // checkcast leaves the operand stack unaffected
+            // instanceof returns 0 if the objectRef is null
+            //
+            TR::LabelSymbol *nullTargetLabel = isCheckCast ? fallThruLabel : notCastableDoNotCacheLabel;
+            generateLabelInstruction(TR::InstOpCode::JE4, node, nullTargetLabel, cg);
+            }


Does TR::icall corresponds directly to Java Class.isAssignableFrom? Will objectNode always be non null if the node is TR::icall? The reason I'm asking is that If cls is null, NullPointerException should be thrown.

a7ehuo · 2026-02-22T18:48:14Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+         TR::Register *scratchReg2 = NULL;
+         TR::Register *scratchReg3 = NULL;
+
+         bool use64BitClasses = cg->comp()->target().is64Bit() && !TR::Compiler->om.generateCompressedObjectHeaders();


The check cg->comp()->target().is64Bit() is redundant since this whole case 2 are wrapped under it

else if (!disableInlineArrayExactCastClass && !isRelocatableCompile && cg->comp()->target().is64Bit())

a7ehuo · 2026-02-22T19:06:29Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+         if (!scratchReg)
+            scratchReg = cg->allocateRegister();
+
+         generateRegMemInstruction(TR::InstOpCode::LRegMem(), node, scratchReg,


Does it need to consider if use64BitClasses is true or false when generating TR::InstOpCode::LRegMem?

a7ehuo · 2026-02-22T19:43:25Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+            if (IS_32BIT_SIGNED(componentClazzAddress))
+               {
+               // TODO: Need a relocation for componentClazz
+               generateRegImmInstruction(TR::InstOpCode::CMPRegImm4(), node, objectClassLeafReg, (int32_t)componentClazzAddress, cg);
+               }
+            else
+               {
+               // TODO: Need a relocation for componentClazz
+               generateRegImm64Instruction(TR::InstOpCode::MOV8RegImm64, node, scratchReg, componentClazzAddress, cg);
+               generateRegRegInstruction(TR::InstOpCode::CMPRegReg(), node, objectClassLeafReg, scratchReg, cg);
+               }


Should it explicitly consider use64BitClasses being true and false cases here?

a7ehuo · 2026-02-22T20:25:49Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+            intptr_t castClassArity = ((J9ArrayClass*)clazz)->arity;
+
+            generateRegImmInstruction(TR::InstOpCode::CMPRegImm4(), node, scratchReg, (int32_t)castClassArity, cg);


Is it necessary to cast ((J9ArrayClass*)clazz)->arity from UDATA → intptr_t → int32_t? Should it be cast directly from UDATA → int32_t?

a7ehuo · 2026-02-23T16:15:40Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+#if defined(J9VM_OPT_VALHALLA_FLATTENABLE_VALUE_TYPES)
+            if (J9_IS_J9ARRAYCLASS_NULL_RESTRICTED(castClass))
+               {
+               static_assert(J9ClassArrayIsNullRestricted == 0x2000000, "Cannot do simple bit test for J9ClassArrayIsNullRestricted");


The assert is currently written to require the value to be 0x2000000, but the assert message is unclear what is expected. Should it be updated to something like expect J9ClassArrayIsNullRestricted == 0x2000000 or J9ClassArrayIsNullRestricted must be 0x2000000 to be explicit?

a7ehuo · 2026-02-23T16:27:09Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+      TR::Register *objectClassReg,
+      uintptr_t clazzAddress,
+      bool use64BitClasses,
+      TR::Register *scratchReg,


scratchReg is not used in this function.

a7ehuo · 2026-02-23T16:36:26Z

runtime/compiler/x/codegen/J9TreeEvaluator.cpp

+             }
+         else
+             {
+             // TODO: process out of line, but could just throw a ClassCastException instead


This TODO: process out of line ... comment sounds a bit ambiguous to me since processing out of line is already happening. Do you mean for checkcast failure, instead of punting to the full helper, generate a dedicated OOL to throw ClassCastException? If so, could the TODO comment be updated to be more accurate?

0xdaryl added 2 commits February 20, 2026 09:25

Inline instanceof checks when cast class is [jlO

4b541a0

Extend the current x86 checkcast optimization when the cast class is a java/lang/Object array to handle instanceof and isAssignableFrom() cases. Signed-off-by: Daryl Maier <maier@ca.ibm.com>

0xdaryl added comp:jit arch:x86 labels Feb 20, 2026

a7ehuo reviewed Feb 20, 2026

View reviewed changes

a7ehuo reviewed Feb 23, 2026

View reviewed changes

		intptr_t castClassArity = ((J9ArrayClass*)clazz)->arity;

		generateRegImmInstruction(TR::InstOpCode::CMPRegImm4(), node, scratchReg, (int32_t)castClassArity, cg);

Conversation

0xdaryl commented Feb 20, 2026

Uh oh!

0xdaryl commented Feb 20, 2026

Uh oh!

a7ehuo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7ehuo Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vijaysun-omr commented Feb 21, 2026

Uh oh!

a7ehuo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7ehuo Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

a7ehuo Feb 20, 2026 •

edited

Loading

a7ehuo Feb 22, 2026 •

edited

Loading