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

Fix ValueTypeArrayTestsJIT test failures #19913

Open
theresa-m opened this issue Jul 24, 2024 · 6 comments
Open

Fix ValueTypeArrayTestsJIT test failures #19913

theresa-m opened this issue Jul 24, 2024 · 6 comments
Labels
comp:jit project:valhalla Used to track Project Valhalla related work

Comments

@theresa-m
Copy link
Contributor

Once #19911 is merged ValueTypeArrayTests can create null restricted arrays.

There are a few test failures in the JIT variations. I suspect most of them are related to the change from NullPointerException to ArrayStoreException when null is attempted to be set to a null restricted array element.

fyi @a7ehuo @hzongaro

@theresa-m theresa-m added comp:jit project:valhalla Used to track Project Valhalla related work labels Jul 24, 2024
@theresa-m theresa-m assigned theresa-m and unassigned theresa-m Aug 1, 2024
@theresa-m
Copy link
Contributor Author

I didn't realize cnathelper.cpp is vm code, I will make updates there and see how many tests are resolved.

@hzongaro
Copy link
Member

@theresa-m, today the JIT tests whether an array is null-restricted by checking whether the J9ClassIsPrimitiveValueType bit is set for the classFlags of the component type of the array. If I understand the changes in #19911 correctly, the JIT will now need to test the J9ClassArrayIsNullRestricted bit of the classFlags of the class of the array itself instead. Is that correct?

@theresa-m
Copy link
Contributor Author

@theresa-m, today the JIT tests whether an array is null-restricted by checking whether the J9ClassIsPrimitiveValueType bit is set for the classFlags of the component type of the array. If I understand the changes in #19911 correctly, the JIT will now need to test the J9ClassArrayIsNullRestricted bit of the classFlags of the class of the array itself instead. Is that correct?

That is correct. Also a J9Class may now have two J9ArrayClass structures, one is the arrayClass field which is always a nullable array and a nullRestrictedArrayClass which will have the J9ClassArrayIsNullRestricted field set.

a7ehuo added a commit to a7ehuo/omr that referenced this issue Sep 4, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19914, eclipse-openj9/openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 4, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 4, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Sep 24, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19914, eclipse-openj9/openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 24, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 24, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 24, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 24, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Sep 24, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19914, eclipse-openj9/openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Sep 24, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Sep 24, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19914, eclipse-openj9/openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 3, 2024
(1) Add recognized method
`jdk/internal/value/ValueClass.newArrayInstance`

(2)
Add VP fixed class constraint if `ValueClass.newArrayInstance`
creates null restricted array

(3)
Rename `isArrayCompTypePrimitiveValueType`
to `isArrayNullRestricted` to reflect the updated logic

Update `isArrayNullRestricted` on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19914, eclipse-openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Oct 3, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19914, eclipse-openj9/openj9#19913

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 8, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Oct 8, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19913, eclipse-openj9/openj9#19914

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Oct 8, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19913, eclipse-openj9/openj9#19914

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 8, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 15, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

(6)
Enable previous disabled JIT Valhalla functional tests

Fixes: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/omr that referenced this issue Oct 17, 2024
Load the source array and the destination array from
the saved temp instead of commoning the original nodes
incorrectly across blocks.

Related: eclipse-openj9/openj9#19913, eclipse-openj9/openj9#19914

Signed-off-by: Annabelle Huo <[email protected]>
a7ehuo added a commit to a7ehuo/openj9 that referenced this issue Oct 18, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
@a7ehuo
Copy link
Contributor

a7ehuo commented Oct 18, 2024

An update: #20291 and #20112 together fix this test failure on X86, but not on other platforms such as Z, Power, or Aarch64.

I looked at the failure on Z and here is why it fails:

ValueTypeArrayTests.runTest uses instanceof to check the passed in array arr type. The passed in array is testArrays[3] which is a null restricted array [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;. But the result shows actualArrayKind is IFACE_TYPE, instead of PRIM_TYPE. So the test expect no exception is thrown but the JVM throws ASE because it is storing null into a null restricted array. Hence the test fails the assert check.

	static void runTest(Object[] arr, Object sourceVal, int staticArrayKind, int staticSourceKind) throws Throwable {
		boolean caughtThrowable = false;
		int actualArrayKind = arr instanceof PointPV[]
								? PRIM_TYPE
								: arr instanceof PointV[]
									? VAL_TYPE
									: arr instanceof SomeIface[]
										? IFACE_TYPE
										: OBJ_TYPE;
[ValueTypeArrayTests] [INFO] calling runTest testArrays[3] NullRestrictedArray nullObj kinds[0] OBJ_TYPE 1 kinds[0] OBJ_TYPE 1
[ValueTypeArrayTests] [INFO] --------------------------
[ValueTypeArrayTests] [INFO] runTest staticArrayKind=OBJ_TYPE 1 staticSourceKind=OBJ_TYPE 1
[ValueTypeArrayTests] [INFO] runTest arr: actualArrayKind=IFACE_TYPE 2 sourceVal: actualSourceKind=NULL_REF 0 //<--- wrong actualArrayKind should be PRIM_TYPE 4 since testArrays[3] is NullRestrictedArray
[ValueTypeArrayTests] [INFO] runTest expectedExceptionClass null
[ValueTypeArrayTests] [INFO] assignDispatch arrKind=OBJ_TYPE srcKind=OBJ_TYPE idx=1
[ValueTypeArrayTests] [INFO] runTest caughtThrowable=java.lang.ArrayStoreException
FAILED: testValueTypeArrayAssignments
java.lang.AssertionError: ActualArrayKind == 2; StaticArrayKind == 1; actualSourceKind == 0; staticSourceKind == 1 expected [null] but found [class java.lang.ArrayStoreException]
	at org.testng.Assert.fail(Assert.java:96)
	at org.testng.Assert.failNotEquals(Assert.java:776)
	at org.testng.Assert.assertEqualsImpl(Assert.java:132)
	at org.testng.Assert.assertEquals(Assert.java:118)
	at org.openj9.test.lworld.ValueTypeArrayTests.runTest(ValueTypeArrayTests.java:296)
	at org.openj9.test.lworld.ValueTypeArrayTests.testValueTypeArrayAssignments(ValueTypeArrayTests.java:359)

Here is the problem with the generated instanceof (n49n) code. It compares object class (GPR_0022) with the cast class in GPR_0021which has value 29663232 (0x1C4A000). 0x1C4A000 is the nullable array class [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;. The null restricted array class for [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV; is 0x1C90A00. The static address 0x1C4A000 for symRef #404 for loadaddr(n48n) is created in loadClassObjectForTypeTest based on cpIndex 24.

@theresa-m @hangshao0 It's a bit unclear to me here is that: Should the cpIndex that the JIT gets here point to the null restricted array class for [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;? I'm not sure if the cpIndex 24 is wrong in bytecode here, or if resolveClassRef doesn't return the correct class object for cpIndex 24. resolveClassRef is what the JIT calls the VM to get the class object based on cp index here.

n53n      ificmpeq --> block_4 BBStart at n1n ()
n49n        instanceof  jitInstanceOf[#87  helper Method] [flags 0x400 0x0 ]
n47n          aload  arr<parm 0 [Ljava/lang/Object;>[#399  Parm] [flags 0x40000107 0x0 ]
n48n          loadaddr  [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;[#404  Static]
n50n        iconst 0 (X==0 X>=0 X<=0 )
 [     0x3ff8167e330]	                       LGHI    GPR_0020,0x1	
 [     0x3ff8167e600]	                       LGFI    GPR_0021,29663232	 //<---- nullable array class
 [     0x3ff8167e740]	                       LTGR    &GPR_0018,&GPR_0018	
 [     0x3ff8167e820]	                       BRC     BH(0x8), Label L0034	
 [     0x3ff8167eac0]	                       LLZRGF  GPR_0022,#460 0(&GPR_0018) //<--- Loading Object Class from object in GPR_0018
 [     0x3ff8167ebe0]	                       CGRJ    GPR_0021,GPR_0022,Label L0032,BH(mask=0x8), 	 ; ClassEqualityTest //<--- comparing object class from GPR_0022 with cast class in GPR_0021
       +------------- Byte Code Index
        |  +-------------------- OpCode
        |  |                        +------------- First Field
        |  |                        |     +------------- Branch Target
        |  |                        |     |      +------- Const Pool Index
        |  |                        |     |      |    +------------- Constant
        |  |                        |     |      |    |
        V  V                        V     V      V    V

        0, JBiconst0            
        1, JBistore                 4
        3, JBaload0             
        4, JBinstanceof                         24

@hangshao0
Copy link
Contributor

hangshao0 commented Oct 21, 2024

It's a bit unclear to me here is that: Should the cpIndex that the JIT gets here point to the null restricted array class for [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;? I'm not sure if the cpIndex 24 is wrong in bytecode here, or if resolveClassRef doesn't return the correct class object for cpIndex 24.

In the current version of Valhalla, the nullable array and null restricted array share the same constant pool class entry (both [Lorg/openj9/test/lworld/ValueTypeArrayTests$PointPV;). resolveClassRef always returns the nullable J9ArrayClass. We won't be able to distinguish them from the constant pool until CheckedType constants is introduced (null restricted CheckedType vs nullable class CheckedType).

Rather than relying on constant pool, not sure if JIT can get the J9Class/J9ArrayClass from the object itself.

@a7ehuo
Copy link
Contributor

a7ehuo commented Oct 21, 2024

Thanks for the clarification @hangshao0! After some offline discussion with @hzongaro, the JIT will wait for CheckedType constants to be ready first.

rmnattas pushed a commit to rmnattas/openj9 that referenced this issue Nov 1, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
zl-wang pushed a commit to zl-wang/openj9 that referenced this issue Nov 11, 2024
(1) Add recognized methods from jdk/internal/value/ValueClass
and jdk/internal/value/NullRestrictedCheckedType

(2)
Add VP fixed class constraint if ValueClass.newArrayInstance
creates null restricted array using NullRestrictedCheckedType

(3)
Rename isArrayCompTypePrimitiveValueType
to isArrayNullRestricted to reflect the updated logic

Update isArrayNullRestricted on how to determine
whether or not an array is a null restricted array

(4)
Disable transformation based on type hint which is no longer
sufficient enough to decide whether or not the array is null
restricted or not

(5)
If flattenable arrays are enabled, do not create fixed class
constraint for parm array class based on signature since
both nullable and null restricted arrays share the same signature

Related: eclipse-openj9#19913, eclipse-openj9#19914, eclipse-openj9#19708

Signed-off-by: Annabelle Huo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:jit project:valhalla Used to track Project Valhalla related work
Projects
None yet
Development

No branches or pull requests

4 participants