On Thu, 27 Apr 2023 12:09:21 GMT, Coleen Phillimore <[email protected]> wrote:
>> src/hotspot/share/oops/constMethodFlags.hpp line 53:
>>
>>> 51: flag(has_type_annotations , 1 << 9) \
>>> 52: flag(has_default_annotations , 1 << 10) \
>>> 53: flag(caller_sensitive , 1 << 11) \
>>
>> Nit: we should consistently use either `x` or `is_x` for `x` in `overpass,
>> caller_sensitive, hidden, scoped, ...`
>
> That's my preference too, but I was trying to not change all the callers to
> is_caller_sensitive, for example, or providing a set of wrappers to change
> the "x" calls to "is_x" calls to the flags interface.
The 'is' in not_x_compilable can be added without too much fanout of lines and
files that I don't want to change, so I added the 'is' to these.
>> src/hotspot/share/oops/method.cpp line 735:
>>
>>> 733: case Bytecodes::_jsr:
>>> 734: if (bcs.dest() < bcs.next_bci()) {
>>> 735: return set_has_loops();
>>
>> I don't understand the new return logic here. The break gets us out of the
>> switch but we are still in the while loop, but the return takes us all the
>> way out. ???
>
> Yes, I had a version of this change where we only let has_loops get set once,
> and this code set it over and over again, which is unnecessary. Once it's
> true, it stays true.
The early break and not resetting has_loops once it's true, saves the atomic
access also.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1179084771
PR Review Comment: https://git.openjdk.org/jdk/pull/13654#discussion_r1179080249
