RFR: 8356390: Rename ResolvedIndyEntry::set_flags to set_has_appendix

[Johan Sjölen]

On Fri, 9 May 2025 05:43:19 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> The `set_flags` function really only sets whether it has an appendix or not, and there's a separate `set_resolution_failed` method just below that also alters the flag. Just rename this to `set_has_appendix`
>
> src/hotspot/share/oops/resolvedIndyEntry.hpp line 120:
> 
>> 118:     u1 old_flags = _flags &  ~(1 << has_appendix_shift);
>> 119:     // Preserve the unaffected bits
>> 120:     _flags = old_flags | new_flags;
> 
> I may be having a mental blank this late in the week, but why do we need to do anything other than:
> 
> _flags |= new_flags;
> 
> ? `new_flags` should at most have one bit set (the appendix bit) and OR'ing with zero preserves all other bits.

Yeah, what you're saying sounds right to me. Let's compare the new, old and your code, together.


u1 new_flags = (has_appendix << has_appendix_shift);  <=> 
u1 new_flags = 0b00000010;

u1 old_flags = _flags & ~(1 << has_appendix_shift) <=>
u1 old_flags = _flags & ~(0b00000010)              <=>
u1 old_flags = _flags & 0b11111101                 <=>
u1old_flags = 0bXXXXXX0X      X = unknown value, whatever was in _flags at that position before

u1 _flags = old_flags | new_flags;   <=>
u1 _flags = 0bXXXXXX0X | 0b00000010; <=>
u1 _flags = 0bXXXXXX1X;


And let's compare that to the old code:


u1 new_flags = (has_appendix << has_appendix_shift); <=>
u1 new_flags = 0b00000010;
u1 _flags = (_flags & 1) | new_flags <=>
u1 _flags = 0x0000000X | 0b00000010  <=>
u1 _flags = 0x0000001X;


So the previous code cleared out all bits except the resolution flag, so if there were more bits in use this method would fail.

Finally, let's look at your version.


u1 new_flags = (has_appendix << has_appendix_shift); <=>
u1 new_flags = 0b00000010;
u1 _flags = _flags | new_flags;
u1 _flags = 0bXXXXXXXX | 0b00000010; <=>
u1 _flags = 0bXXXXXX1X;


That's pretty clearly identical to the new version. Let's skip the implicit `bool` to `1` conversion while we're at it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25092#discussion_r2081174671