[llvm-branch-commits] [llvm] d946f7b - [PowerPC] Don't let optimizeCompareInstr define CR0 while it is still live (#220999)
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Oct 18 05:08:50 PDT 2026
Author: Daniel Moody
Date: 2026-10-18T14:08:24+02:00
New Revision: d946f7b2b214f31525b5a14d88d108e16515bbb3
URL: https://github.com/llvm/llvm-project/commit/d946f7b2b214f31525b5a14d88d108e16515bbb3
DIFF: https://github.com/llvm/llvm-project/commit/d946f7b2b214f31525b5a14d88d108e16515bbb3.diff
LOG: [PowerPC] Don't let optimizeCompareInstr define CR0 while it is still live (#220999)
This replaces the compare-against-zero in
`PPCInstrInfo::optimizeCompareInstr` with the record form of the
instruction defining the compared register only when CR0 is dead at that
point, checked with `MachineBasicBlock::computeRegisterLiveness`
scanning to the end of the block.
Fixes #220996.
## Why
The conversion makes `MI` define CR0. CR0 can still be live there: the
`ANDI_rec_1_{EQ,GT}_BIT` pseudo expansion emits `ANDI_rec` followed by
COPYs of `$cr0eq`/`$cr0gt`, and nothing keeps those copies adjacent to
their `andi.` once other instructions are scheduled in between. A later
compare candidate sitting between an `andi.` and one of its copies gets
converted, clobbers CR0, and the copy then reads the wrong value — the
two flag tests swap conditions.
The existing backward safety scan cannot see this: it only covers the
range between the candidate and the compare's first use (the stale
reader sits after that), and it is skipped entirely when `noSub` is set,
i.e. for every 32-bit compare on PPC64. Un-skipping that scan alone does
not fix the case from the issue (verified); the forward liveness check
does.
## Verification
PowerPC-only builds with assertions on (release/19.x 19.1.7 and main
d8217655d):
- New MIR test `llvm/test/CodeGen/PowerPC/cmp-opt-cr0-live.mir`: fails
on stock `llc` — the conversion happens and `-verify-machineinstrs`
reports `Bad machine code: Using an undefined physical register` for the
stale `%6:crbitrc = COPY $cr0gt` — and passes with this patch.
- `ninja check-llvm-codegen-powerpc`-equivalent (`llvm-lit
test/CodeGen/PowerPC`): identical pass/fail sets with and without the
patch on both bases, apart from the new test. (In my PowerPC-only
environment a handful of tests that need a native default target or
unbuilt utilities fail identically with and without the patch.)
- The C reproducer from the issue and the real-world victim
(libmongocrypt `unicode_fold()`, the case/diacritic folding for MongoDB
Queryable Encryption text search on ppc64le), compiled through the
patched `llc` for `powerpc64le` and run under `qemu-ppc64le`: correct
output.
- The peephole still fires where it is safe: in `unicode_fold`, 2 of the
3 record-form `andi.` conversions remain, only the unsafe one is
dropped.
---
*Disclosure (per the LLVM developer policy on AI-assisted
contributions): the initial analysis, reproduction, and drafting of this
change were done with the assistance of an AI coding tool. I have
reproduced the failure, reviewed every line of the patch, re-run the
verification myself, and am submitting this as the responsible author.*
---------
Co-authored-by: Amy Kwan <amy.kwan1 at ibm.com>
(cherry picked from commit 8ed9dd99e7a8dd2fd6b953e5ddae80566e3b49af)
Added:
llvm/test/CodeGen/PowerPC/cmp-opt-cr0-live.mir
Modified:
llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
index f415a9776da94e..bdaa72b7fe210d 100644
--- a/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
+++ b/llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
@@ -2650,6 +2650,25 @@ bool PPCInstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
if (NewOpC == -1)
return false;
+ // Converting MI to its record form makes it define CR0, which is only legal
+ // if CR0 is dead at that point. CR0 may still be live from an earlier
+ // record-form instruction with a use scheduled after MI: expanding the
+ // ANDI_rec_1_{EQ,GT}_BIT pseudos produces COPYs of CR0 subregisters, and
+ // nothing keeps those adjacent to their `andi.` once other instructions are
+ // scheduled in between. The backward scan above only covers the range
+ // between MI and CmpInstr, so it cannot see such a use. Query CR0's
+ // liveness right after MI, using the block size as the neighborhood so the
+ // query can inspect the remainder of the block, and be conservative about
+ // an unknown answer.
+ const bool IntroducesRecFormInstr = MIOpC != NewOpC;
+ if (IntroducesRecFormInstr) {
+ const MachineBasicBlock *MBB = MI->getParent();
+ if (MBB->computeRegisterLiveness(
+ &RI, PPC::CR0, std::next(MachineBasicBlock::const_iterator(MI)),
+ /*Neighborhood=*/MBB->size()) != MachineBasicBlock::LQR_Dead)
+ return false;
+ }
+
// This transformation should not be performed if `nsw` is missing and is not
// `equalityOnly` comparison. Since if there is overflow, sub_lt, sub_gt in
// CRReg do not reflect correct order. If `equalityOnly` is true, sub_eq in
diff --git a/llvm/test/CodeGen/PowerPC/cmp-opt-cr0-live.mir b/llvm/test/CodeGen/PowerPC/cmp-opt-cr0-live.mir
new file mode 100644
index 00000000000000..3054ddfa6e1770
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/cmp-opt-cr0-live.mir
@@ -0,0 +1,72 @@
+# RUN: llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -run-pass=peephole-opt \
+# RUN: -verify-machineinstrs %s -o - | FileCheck %s
+#
+# optimizeCompareInstr must not convert the RLWINM into a record-form `andi.`
+# while CR0, set by the earlier ANDI_rec, remains live until its use by the
+# later COPY $cr0gt. Doing so swaps the condition bits of the two flag tests.
+# Once CR0 has no further use, the conversion is still performed.
+
+---
+name: cr0_live_across_candidate
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $r3
+
+ ; CHECK-LABEL: name: cr0_live_across_candidate
+ ; CHECK: %3:gprc = RLWINM %0, 0, 30, 30
+ ; CHECK: %4:crrc = CMPWI killed %3, 0
+ %0:gprc = COPY $r3
+ %1:gprc = ANDI_rec %0, 1, implicit-def $cr0
+ %2:crbitrc = COPY $cr0eq
+ %3:gprc = RLWINM %0, 0, 30, 30
+ %4:crrc = CMPWI killed %3, 0
+ %5:crbitrc = COPY %4.sub_eq
+ %6:crbitrc = COPY $cr0gt
+ %7:crbitrc = CRAND %2, %5
+ %8:crbitrc = CRAND %7, %6
+ BC %8, %bb.2
+ B %bb.1
+
+ bb.1:
+ %9:gprc = LI 1
+ $r3 = COPY %9
+ BLR8 implicit $lr8, implicit $rm, implicit $r3
+
+ bb.2:
+ %10:gprc = LI 0
+ $r3 = COPY %10
+ BLR8 implicit $lr8, implicit $rm, implicit $r3
+...
+---
+name: cr0_dead_across_candidate
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $r3
+
+ ; CHECK-LABEL: name: cr0_dead_across_candidate
+ ; CHECK: %4:gprc = ANDI_rec %0, 2, implicit-def $cr0
+ ; CHECK-NEXT: %5:crrc = COPY killed $cr0
+ %0:gprc = COPY $r3
+ %1:gprc = ANDI_rec %0, 1, implicit-def $cr0
+ %2:crbitrc = COPY $cr0eq
+ %3:crbitrc = COPY $cr0gt
+ %4:gprc = RLWINM %0, 0, 30, 30
+ %5:crrc = CMPWI killed %4, 0
+ %6:crbitrc = COPY %5.sub_eq
+ %7:crbitrc = CRAND %2, %6
+ %8:crbitrc = CRAND %7, %3
+ BC %8, %bb.2
+ B %bb.1
+
+ bb.1:
+ %9:gprc = LI 1
+ $r3 = COPY %9
+ BLR8 implicit $lr8, implicit $rm, implicit $r3
+
+ bb.2:
+ %10:gprc = LI 0
+ $r3 = COPY %10
+ BLR8 implicit $lr8, implicit $rm, implicit $r3
+...
More information about the llvm-branch-commits
mailing list