openj9
openj9 copied to clipboard
eclipse-openj9
Set default values for implicit value classes
- Remove Primitive flag that was set for nullrestricted flattening as a temporary workaround (see https://github.com/eclipse-openj9/openj9/pull/18597 for more details)
- Add J9ClassAllowsInitialDefaultValue ram class flag to show class has implicitcreation attribute with default flag set
- Use existing J9ClassContainsUnflattenedFlattenables to flag classes that are not flattened but should have defaults set.
Related https://github.com/eclipse-openj9/openj9/issues/17234
I also did a full run of the lw5 valhalla tests for this change:
What has changed with this pull request:
- ValueTypeTests are now passing, previously there were 4 failures
- ValhallaAttributeTests.testPutFieldNullToNullRestrictedField is failing for variations 4/5/6. This was previously fixed https://github.com/eclipse-openj9/openj9/issues/18742 but is failing for a different reason now. By removing the primitive flag the ram class is no longer generated as a qtype and is not marked as nullrestricted in the jit because this test uses
instead of aconst_init. Likely will be fixed by https://github.com/eclipse-openj9/openj9/pull/18790 - ValueTypeArrayTests. This pull request fixes some test failures here that were related to default values however variations with array flattening enabled are failing. This will be addressed in a future pull request
- I didn't triage the rest of the failing test since they were failing before this change and are mostly jit related, I will take a closer look once vm support is more complete
Passing:
cmdLineTester_unflattenedvaluetypeddrtests cmdLineTester_flattenedvaluetypeddrtests cmdLineTester_flattened32bitRefBackfillTest cmdLineTester_flattened32bitRefBackfillTestJIT ValueTypeTests ValueTypeUnsafeTests ValueTypeOptTests
Failing:
ValhallaAttributeTests ValueTypeTestsJIT ValueTypeArrayTests ValueTypeArrayTestsJIT NullRestrictedTypeOptTests ValueTypeSystemArraycopyTests
FYI @hzongaro @a7ehuo As primitive key word is going to be removed, we won't have a primitive VT class. Instead we will have a class that has default value which can be used as a null restricted type. This PR is adding a flag J9ClassAllowsInitialDefaultValue to J9Class->classFlags indicating such classes. Not sure if JIT needs something similar to J9::ClassEnv::isPrimitiveValueTypeClass() to check this new flag.
Not sure if JIT needs something similar to
J9::ClassEnv::isPrimitiveValueTypeClass()to check this new flag.
Thanks @hangshao0! I think we will create a new API in J9::ClassEnv to query J9ClassAllowsInitialDefaultValue.
