RFR: 8300800: UB: Shift exponent 32 is too large for 32-bit type 'int' [v2]
Kim Barrett
kbarrett at openjdk.org
Mon Aug 12 18:39:33 UTC 2024
On Mon, 12 Aug 2024 12:47:42 GMT, Andrew Dinn <adinn at openjdk.org> wrote:
>> src/hotspot/cpu/aarch64/immediate_aarch64.cpp line 298:
>>
>>> 296: uint64_t or_bits_sub = replicate(or_bit, 1, nbits);
>>> 297: uint64_t and_bits_top = (and_bits_sub << nbits) | ones(nbits);
>>> 298: uint64_t or_bits_top = (UCONST64(0) << nbits) | or_bits_sub;
>>
>> I focused on the UL suffix earlier, and didn't really think about what this is doing. Why are we shifting
>> a zero value at all? This equivalent to `uint64_t or_bits_top = or_bits_sub;`.
>
> I believe this question came up in an earlier thread and was answered by @theRealAph. The shift of zero is there to emphasise continuity of this case with other cases where a non-zero value is shifted i.e. it serves to emphasize/document the connection between this implementation and the algorithm that it embodies.
Thanks for the background. It still looks weird and I can't unsee it now. But a comment might be almost
as intrusive to readability. So okay.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20530#discussion_r1714219217
More information about the hotspot-compiler-dev
mailing list