[lworld] RFR: 8288135: [lworld] Implement HotSpot flag -XX:+EnablePrimitiveClasses [v10]

[David Simms]

On Mon, 3 Oct 2022 14:09:29 GMT, Frederic Parain <fparain at openjdk.org> wrote:

>> David Simms has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 22 commits:
>> 
>>  - Merge branch 'lworld' into 8288135
>>  - Extra assertion info for tracking spurious issue
>>  - supports_inline_types() refers to classfile and is separate from Enable flags
>>  - Further testing explicity using flags
>>  - Disable EnablePrimitiveClasses by default
>>  - Explictly flag more tests
>>  - Merge branch 'lworld' into 8288135
>>  - jdk_valhalla tests
>>  - Merge branch 'lworld' into 8288135
>>  - Further test fixes
>>  - ... and 12 more: https://git.openjdk.org/valhalla/compare/951544ff...1853e0c6
>
> src/hotspot/share/classfile/classFileParser.cpp line 3328:
> 
>> 3326:       recognized_modifiers |= JVM_ACC_MODULE;
>> 3327:     }
>> 3328:     // JVM_ACC_VALUE, JVM_ACC_PRIMITIVE, and JVM_ACC_IDENTITY are defined depending on version and feature flag
> 
> The implementation doesn't seem to match the comment nor the intent of the patch. JVM_ACC_VALUE and JVM_ACC_IDENTITY should be controlled by the class file version (tested with supports_inline_types()), but JVM_ACC_PRIMITIVE should be controlled by the class file version and the EnablePrimitiveClasses flag.

Agree the comment doesn't quite match. Let me check the behavior is correct:

Currently (in this patch); 
- if the classfile is new enough to be aware of the existence (`supports_inline_types()`) of new flags, the are placed in the recognized set, and later feature checks produce errors to use the appropriate switches
- older classfiles we simply ignore modifiers that were not defined at the time of that version, if one took a new primitive, and downgraded it's version, the VM will quietly ignore these modifiers (but probably take issue with other classfile features, like static factory with special naming)
- `ACC_SUPER `and `ACC_IDENTITY` overlap requires special handling based on both version and feature flag (`EnableValhalla`)

Is this your view too ?

-------------

PR: https://git.openjdk.org/valhalla/pull/727