RFR: 8307352: AARCH64: Improve itable_stub [v8]

Boris Ulasevich bulasevich at openjdk.org
Mon Aug 28 13:44:20 UTC 2023


On Sat, 26 Aug 2023 20:38:58 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 1231:
>> 
>>> 1229:   // itableOffsetEntry[] itable = recv_klass + Klass::vtable_start_offset() + sizeof(vtableEntry) * recv_klass->_vtable_len;
>>> 1230:   // temp_itbl_klass = itable[0]._interface;
>>> 1231:   ldr(temp_itbl_klass, Address(recv_klass, scan_temp, Address::lsl(exact_log2(vtableEntry::size_in_bytes()))));
>> 
>> The lsl shift in `ldr` must be 3.
>> Can we have a static assert checking that? There is an assert in `Address::encode` which will catch the issue in debug builds. As `vtableEntry::size_in_bytes()` is statically known we can catch the issue during compilation.
>
> I'm very surprised that isn't a guarantee. I had to look to be sure. I'd simply change `assert(ext().shift() <= 0 || ext().shift() == (int)size, "bad shift");` to a `guarantee`.

I want to avoid modifying the assembler_aarch64.hpp file within this change, so I add guarantee in lookup_interface_method.
By the way, [assembler_aarch64.hpp](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/assembler_aarch64.hpp) contains 100+ assert calls and 34 guarantee calls. I think more cases can be changed, although I am not sure what the rule is for using this or that call.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13792#discussion_r1307442927


More information about the hotspot-dev mailing list