[lworld+fp16] RFR: 8308363: Initial compiler support for FP16 scalar operations. [v4]

[Jatin Bhateja]

On Thu, 24 Aug 2023 08:38:42 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Addressing offline review comments from Sandhya, new IR test addition.
>
> src/hotspot/share/classfile/classFileParser.cpp line 5176:
> 
>> 5174:       // Can't enable this check fully until JDK upgrades the bytecode generators (TODO: JDK-8270852).
>> 5175:       // For now, compare to class file version 51 so old verifier doesn't see Q signatures.
>> 5176:       if ( (_major_version < 51 /* CONSTANT_CLASS_DESCRIPTORS */ ) || (!EnablePrimitiveClasses && !is_jdk_internal_class_sig(signature))) {
> 
> The last condition `(!EnablePrimitiveClasses && !is_jdk_internal_class_sig(signature))` seems not right to me. Assume there is a jdk internal primitive class defined , and `EnablePrimitiveClasses` is disabled, then result of `(!EnablePrimitiveClasses && !is_jdk_internal_class_sig(signature))` is `false` (i.e. `true && false`), then the followed error is not printed if the jdk version matches >= 51. But it should report the error since `EnablePrimitiveClasses` is closed.  So it should be:
> 
> if ( (_major_version < 51 /* CONSTANT_CLASS_DESCRIPTORS */ ) || !EnablePrimitiveClasses || !is_jdk_internal_class_sig(signature))
> 
> right?

Idea here is to relax the need to use an explicit JVM flag -XX:+EnablePrimtiiveClasses for primitive classes known to VM.

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/848#discussion_r1311394730