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