RFR: 8339113: AccessFlags can be u2 in metadata [v12]

[Coleen Phillimore]

On Mon, 6 Jan 2025 16:46:25 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove unused SA function.
>
> src/hotspot/share/interpreter/linkResolver.cpp line 586:
> 
>> 584:     // We need to change "protected" to "public".
>> 585:     assert(flags.is_protected(), "clone not protected?");
>> 586:     u2 new_flags = flags.as_unsigned_short();
> 
> Nit: Should this also be replaced with `as_method_flags()`?

Thanks Serguei, I replaced this one and a couple of as_field_flags() so that as_unsigned_short() is more limited to the cases where we don't want masking.

> src/hotspot/share/opto/memnode.cpp line 1985:
> 
>> 1983:     // The field is Klass::_access_flags.  Return its (constant) value.
>> 1984:     // (Folds up the 2nd indirection in Reflection.getClassAccessFlags(aClassConstant).)
>> 1985:     assert(this->Opcode() == Op_LoadUS, "must load an unsigned short from _access_flags");
> 
> Nit: This can be unified with line 1979 and also get rid of `this->`.

1979 and 1985 are in different branches of an if statement (address of modifier flags vs access flags) so needs to be repeated.  But I did remove the this->

> src/hotspot/share/prims/jvm.cpp line 2472:
> 
>> 2470:   u2 field_access_flags = InstanceKlass::cast(k)->field_access_flags(field_index);
>> 2471:   // This & should be unnecessary.
>> 2472:   assert((field_access_flags & JVM_RECOGNIZED_FIELD_MODIFIERS) == field_access_flags, "already masked");
> 
> Nit: Yes, it is better to remove the lines: 2471-2472.

fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22246#discussion_r1904829376
PR Review Comment: https://git.openjdk.org/jdk/pull/22246#discussion_r1904830369
PR Review Comment: https://git.openjdk.org/jdk/pull/22246#discussion_r1904829864