[llvm] 08e6e14 - [GlobalISel] Fix sign-extended byte mask in lowerBswap (#199387)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 12 18:41:41 PDT 2026
Author: Ilpo Ruotsalainen
Date: 2026-06-13T01:41:36Z
New Revision: 08e6e1476b4d3273f95ef715391154be0a6881bc
URL: https://github.com/llvm/llvm-project/commit/08e6e1476b4d3273f95ef715391154be0a6881bc
DIFF: https://github.com/llvm/llvm-project/commit/08e6e1476b4d3273f95ef715391154be0a6881bc.diff
LOG: [GlobalISel] Fix sign-extended byte mask in lowerBswap (#199387)
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.
Use `APInt::getBitsSet` to construct the mask, matching the file's other
bit-range mask constructions.
Fixes #199386
Added:
Modified:
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bitreverse-rv64.mir
llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir
llvm/test/CodeGen/RISCV/GlobalISel/rv64zbb.ll
llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
Removed:
################################################################################
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