[GlobalISel] Fix sign-extended byte mask in lowerBswap by lonemeow · Pull Request #199387 · llvm/llvm-project

lonemeow · 2026-05-24T01:05:37Z

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

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 llvm#199386

github-actions · 2026-05-24T01:05:53Z

Hello @lonemeow 👋

Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.

All contributions to LLVM must follow our LLVM AI Tool Use Policy. In particular, if you used AI while working on this PR, remember to add a note to the PR description.
The LLVM Code-Review Policy and Practices document contains practical information about the PR process, including how patches are reviewed and accepted, and who can review a PR.
Our LLVM Developer Policy describes our expectations for code quality, commit summaries and contains notes on our CI system.

Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description.

Frequently asked questions

How do I add reviewers?

This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically.

You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using @ followed by their GitHub username.

What if there are no comments?

If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers.

Are any special GitHub settings required to contribute to LLVM?

We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details.

If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse.

Thank you,
The LLVM Community

llvmorg-github-actions · 2026-05-24T01:06:24Z

@llvm/pr-subscribers-llvm-globalisel

Author: Ilpo Ruotsalainen (lonemeow)

Changes

The per-byte mask in LegalizerHelper::lowerBswap was constructed via

APInt APMask(SizeInBytes * 8, 0xFF &lt;&lt; (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

Full diff: https://github.com/llvm/llvm-project/pull/199387.diff

3 Files Affected:

(modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+1-1)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir (+1-1)
(modified) llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp (+28)

diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index a200aa29f95ca..0cf101e8420dd 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -10042,7 +10042,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-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/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();

llvmorg-github-actions · 2026-05-24T01:06:24Z

@llvm/pr-subscribers-backend-risc-v

Author: Ilpo Ruotsalainen (lonemeow)

Changes

The per-byte mask in LegalizerHelper::lowerBswap was constructed via

APInt APMask(SizeInBytes * 8, 0xFF &lt;&lt; (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

Full diff: https://github.com/llvm/llvm-project/pull/199387.diff

3 Files Affected:

(modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+1-1)
(modified) llvm/test/CodeGen/RISCV/GlobalISel/legalizer/legalize-bswap-rv64.mir (+1-1)
(modified) llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp (+28)

diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index a200aa29f95ca..0cf101e8420dd 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -10042,7 +10042,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-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/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();

topperc

LGTM

llvmorg-github-actions Bot added backend:RISC-V llvm:globalisel labels May 24, 2026

topperc approved these changes May 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GlobalISel] Fix sign-extended byte mask in lowerBswap#199387

[GlobalISel] Fix sign-extended byte mask in lowerBswap#199387
lonemeow wants to merge 1 commit into
llvm:mainfrom
lonemeow:fix-lowerbswap-sign-extended-mask

lonemeow commented May 24, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Uh oh!

llvmorg-github-actions Bot commented May 24, 2026

Uh oh!

llvmorg-github-actions Bot commented May 24, 2026

Uh oh!

topperc left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lonemeow commented May 24, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Frequently asked questions

Uh oh!

llvmorg-github-actions Bot commented May 24, 2026

Uh oh!

llvmorg-github-actions Bot commented May 24, 2026

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants