RFR: 8256436: AArch64: Fix undefined behavior for signed right shift in assembler
Andrew Haley
aph at openjdk.java.net
Fri Nov 20 09:06:08 UTC 2020
On Thu, 19 Nov 2020 05:21:39 GMT, Xiaohong Gong <xgong at openjdk.org> wrote:
> Right shift a signed negative value is implementation-defined in C++ (see [1]). It's better to avoid the signed right shift operations,
> and use the unsigned right shift instead.
>
> [1] https://docs.microsoft.com/en-us/cpp/cpp/left-shift-and-right-shift-operators-input-and-output?view=msvc-160&viewFallbackFrom=vs-2019
>
> Tested jtreg langtools:tier1, hotspot:hotspot_all_no_apps and jdk:jdk_core, and all tests pass without new failures.
Changes requested by aph (Reviewer).
src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3212:
> 3210: } else if (T != B && imm16 <= 32512 && imm16 >= -32768 && (imm16 & 0xff) == 0) {
> 3211: sh = 1;
> 3212: imm = (imm >> 8);
I'm very skeptical about the need for this. Every AArch64 compiler of which I'm aware treats signed right shift as well defined, and any incoming AArch64 compiler would need to do so in order to be compatible.
src/hotspot/cpu/aarch64/assembler_aarch64.hpp line 3219:
> 3217: imm &= mask;
> 3218: f(0b00100101, 31, 24), f(T, 23, 22), f(0b11100011, 21, 14);
> 3219: f(sh, 13), f(imm, 12, 5), rf(Zd, 0);
imm here is a "signed immediate in the range -128 to 127," and it would be perverse to treat it as an unsigned value in the assembler.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1307
More information about the hotspot-compiler-dev
mailing list