[llvm] [GlobalISel] Fix sign-extended byte mask in lowerBswap (PR #199387)
Ilpo Ruotsalainen via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 12 18:07:57 PDT 2026
https://github.com/lonemeow updated https://github.com/llvm/llvm-project/pull/199387
>From 8fe852492196251b49c4a3832b48f423cc0b6e28 Mon Sep 17 00:00:00 2001
From: Ilpo Ruotsalainen <lonewolf at iki.fi>
Date: Sat, 23 May 2026 18:03:45 -0700
Subject: [PATCH] [GlobalISel] Fix sign-extended byte mask in lowerBswap
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The per-byte mask in `LegalizerHelper::lowerBswap` was constructed via
```
APInt APMask(SizeInBytes * 8, 0xFF << (i * 8));
```
where `0xFF << (i * 8)` is evaluated as a signed `int`. For `i*8 >= 24`
(byte-3 mask of an s64 G_BSWAP) the value `0xFF000000` does not fit in a
positive 32-bit `int`; the conversion to signed `int` is
implementation-defined under C++17 (UB under C++11, fully defined under
C++20) and on two's-complement targets produces `-16777216`. The
modular conversion to `uint64_t` in the `APInt` constructor then
materializes that negative `int` as `0xFFFFFFFFFF000000` — the intended
mask was `0x00000000FF000000`. The over-wide mask preserved bytes 4-7
of the source where only byte 3 was intended, and the spurious bytes
propagated through the subsequent shift/OR chain.
This is a runtime miscompile on any GlobalISel target that legalizes
`G_BSWAP` via `lowerBswap` at a type with `SizeInBytes >= 8` — currently
RV64 without Zbb/Zbkb. Native-bswap targets (AArch64, X86) and
`maxScalar(0,s32).lower()`-style RISC-V rules are unaffected because the
buggy loop iteration is never reached.
Use `APInt::getBitsSet` to construct the mask, matching the file's other
bit-range mask constructions.
Fixes #199386
---
.../CodeGen/GlobalISel/LegalizerHelper.cpp | 2 +-
.../legalizer/legalize-bitreverse-rv64.mir | 2 +-
.../legalizer/legalize-bswap-rv64.mir | 2 +-
llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll | 25 +++++++++--------
.../GlobalISel/LegalizerHelperTest.cpp | 28 +++++++++++++++++++
5 files changed, 44 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index 7c1333e127deb..613ee2b705b8d 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -10041,7 +10041,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::lowerBswap(MachineInstr &MI) {
// Set i-th high/low byte in Res to i-th low/high byte from Src.
for (unsigned i = 1; i < SizeInBytes / 2; ++i) {
// AND with Mask leaves byte i unchanged and sets remaining bytes to 0.
- APInt APMask(SizeInBytes * 8, 0xFF << (i * 8));
+ APInt APMask = APInt::getBitsSet(SizeInBytes * 8, i * 8, i * 8 + 8);
auto Mask = MIRBuilder.buildConstant(Ty, APMask);
auto ShiftAmt = MIRBuilder.buildConstant(Ty, BaseShiftAmt - 16 * i);
// Low byte shifted left to place of high byte: (Src & Mask) << ShiftAmt.
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
index 31c9b2c0dbccf..14cd32020d301 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
@@ -189,7 +189,7 @@ body: |
; CHECK-NEXT: [[LSHR2:%[0-9]+]]:_(s64) = G_LSHR [[COPY]], [[C4]](s64)
; CHECK-NEXT: [[AND3:%[0-9]+]]:_(s64) = G_AND [[LSHR2]], [[C3]]
; CHECK-NEXT: [[OR4:%[0-9]+]]:_(s64) = G_OR [[OR3]], [[AND3]]
- ; CHECK-NEXT: [[C5:%[0-9]+]]:_(s64) = G_CONSTANT i64 -16777216
+ ; CHECK-NEXT: [[C5:%[0-9]+]]:_(s64) = G_CONSTANT i64 4278190080
; CHECK-NEXT: [[C6:%[0-9]+]]:_(s64) = G_CONSTANT i64 8
; CHECK-NEXT: [[AND4:%[0-9]+]]:_(s64) = G_AND [[COPY]], [[C5]]
; CHECK-NEXT: [[SHL3:%[0-9]+]]:_(s64) = G_SHL [[AND4]], [[C6]](s64)
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir
index b5d28765889e1..be4c8e7c2a12e 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir
@@ -117,7 +117,7 @@ body: |
; RV64I-NEXT: [[LSHR2:%[0-9]+]]:_(s64) = G_LSHR [[COPY]], [[C4]](s64)
; RV64I-NEXT: [[AND3:%[0-9]+]]:_(s64) = G_AND [[LSHR2]], [[C3]]
; RV64I-NEXT: [[OR4:%[0-9]+]]:_(s64) = G_OR [[OR3]], [[AND3]]
- ; RV64I-NEXT: [[C5:%[0-9]+]]:_(s64) = G_CONSTANT i64 -16777216
+ ; RV64I-NEXT: [[C5:%[0-9]+]]:_(s64) = G_CONSTANT i64 4278190080
; RV64I-NEXT: [[C6:%[0-9]+]]:_(s64) = G_CONSTANT i64 8
; RV64I-NEXT: [[AND4:%[0-9]+]]:_(s64) = G_AND [[COPY]], [[C5]]
; RV64I-NEXT: [[SHL3:%[0-9]+]]:_(s64) = G_SHL [[AND4]], [[C6]](s64)
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll b/llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll
index cae0936a20b0b..969a5bc71be7b 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll
@@ -1164,24 +1164,25 @@ define i64 @bswap_i64(i64 %a) {
; RV64I-NEXT: addi a1, a1, -256
; RV64I-NEXT: or a2, a3, a2
; RV64I-NEXT: and a3, a0, a1
-; RV64I-NEXT: slli a3, a3, 40
; RV64I-NEXT: srli a4, a0, 40
+; RV64I-NEXT: slli a3, a3, 40
; RV64I-NEXT: and a1, a4, a1
-; RV64I-NEXT: lui a4, 4080
; RV64I-NEXT: or a1, a2, a1
-; RV64I-NEXT: and a2, a0, a4
+; RV64I-NEXT: lui a2, 4080
; RV64I-NEXT: or a1, a1, a3
-; RV64I-NEXT: slli a2, a2, 24
-; RV64I-NEXT: srli a3, a0, 24
-; RV64I-NEXT: lui a5, 1044480
-; RV64I-NEXT: and a3, a3, a4
-; RV64I-NEXT: and a4, a0, a5
-; RV64I-NEXT: or a2, a2, a3
-; RV64I-NEXT: slli a4, a4, 8
-; RV64I-NEXT: or a2, a2, a4
+; RV64I-NEXT: and a3, a0, a2
+; RV64I-NEXT: slli a3, a3, 24
+; RV64I-NEXT: li a4, 255
+; RV64I-NEXT: srli a5, a0, 24
+; RV64I-NEXT: slli a4, a4, 24
+; RV64I-NEXT: and a2, a5, a2
+; RV64I-NEXT: and a5, a0, a4
+; RV64I-NEXT: or a2, a3, a2
+; RV64I-NEXT: slli a5, a5, 8
+; RV64I-NEXT: or a2, a2, a5
; RV64I-NEXT: srli a0, a0, 8
; RV64I-NEXT: or a1, a1, a2
-; RV64I-NEXT: and a0, a0, a5
+; RV64I-NEXT: and a0, a0, a4
; RV64I-NEXT: or a0, a1, a0
; RV64I-NEXT: ret
;
diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
index 809d8880faaed..0e9f571ed5175 100644
--- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
@@ -3373,6 +3373,34 @@ TEST_F(AArch64GISelMITest, LowerBSWAP) {
EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
}
+// Test lowering of scalar s64 G_BSWAP.
+TEST_F(AArch64GISelMITest, LowerBSWAPScalarS64) {
+ setUp();
+ if (!TM)
+ GTEST_SKIP();
+
+ DefineLegalizerInfo(A, {});
+
+ auto BSwap = B.buildBSwap(LLT::scalar(64), Copies[0]);
+ AInfo Info(MF->getSubtarget());
+ DummyGISelObserver Observer;
+ LegalizerHelper Helper(*MF, Info, Observer, B, &*LibcallLowering);
+ EXPECT_EQ(LegalizerHelper::LegalizeResult::Legalized,
+ Helper.lower(*BSwap, 0, LLT()));
+
+ // Per-byte AND masks the lowering loop emits for i = 1, 2, 3:
+ // 0x000000000000FF00 = 65280
+ // 0x0000000000FF0000 = 16711680
+ // 0x00000000FF000000 = 4278190080
+ auto CheckStr = R"(
+ CHECK-DAG: G_CONSTANT i64 65280
+ CHECK-DAG: G_CONSTANT i64 16711680
+ CHECK-DAG: G_CONSTANT i64 4278190080
+ )";
+
+ EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF;
+}
+
// Test lowering of G_SDIVREM into G_SDIV and G_SREM
TEST_F(AArch64GISelMITest, LowerSDIVREM) {
setUp();
More information about the llvm-commits
mailing list