llvm / llvm/llvm-project

[PowerPC] optimizeCompareInstr introduces a CR0 def while CR0 is still live (miscompile, machine verifier failure)

Open
#220,996 1 comment 0 reactions 0 assignees View on GitHub
confirmed crash-on-valid
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

# [PowerPC] optimizeCompareInstr introduces a CR0 def while CR0 is still live (miscompile, machine verifier failure)

## Summary

`PPCInstrInfo::optimizeCompareInstr` replaces a compare-with-zero by converting the
instruction that defines the compared register into its record form, which defines
CR0. It does not check that CR0 is dead at that point. When an earlier record-form
instruction set CR0 and one of its readers is scheduled after the converted
instruction, the new definition clobbers the value the reader expects.

Observed as a real miscompile: two flag bits tested via `andi.` end up with their
EQ/GT condition bits swapped. Reproduces on 14.x, 18.x, 19.1.7 and 23.1.0
(`powerpc64le-linux-gnu`, `-O2` and above, any `-mcpu`). `-O0`/`-O1` and
`-mllvm -disable-ppc-cmp-opt` produce correct code.

## MIR reproducer (fails the machine verifier on stock llc)

```
llc -mtriple=powerpc64le-unknown-linux-gnu -mcpu=pwr8 -run-pass=peephole-opt \
-verify-machineinstrs cmp-opt-cr0-live.mir -o -
```

```
---
name: cr0_live_across_candidate
tracksRegLiveness: true
body: |
bb.0:
liveins: $r3
%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
...
```

Stock `llc` rewrites `%3 = RLWINM ...; %4 = CMPWI %3, 0` into
`%3 = ANDI_rec %0, 2, implicit-def $cr0; %4 = COPY $cr0`, so the later
`%6 = COPY $cr0gt` now reads the CR0 of `%0 & 2` instead of `%0 & 1`, and the
verifier reports:

```
*** Bad machine code: Using an undefined physical register ***
- instruction: %6:crbitrc = COPY $cr0gt
LLVM ERROR: Found 1 machine code errors.
```

## How this arises from C

The pre-RA `COPY $cr0eq` / `COPY $cr0gt` pairs come from the expansion of the
`ANDI_rec_1_EQ_BIT` / `ANDI_rec_1_GT_BIT` pseudos in
`PPCTargetLowering::EmitInstrWithCustomInserter`; nothing keeps those copies
adjacent to their `andi.`, so a later unrelated `(x & 2) == 0` test can be
scheduled between them. Minimal C (two TUs so the helpers stay opaque):

```c
// repro.c
#include
#include
#include
#include
#include
enum { LOWER = 1, DIACRITICS = 2 };
uint32_t to_lower_slow(uint32_t cp);
uint32_t strip_slow(uint32_t cp);
__attribute__((noinline)) bool fold(const uint8_t *in, size_t len, unsigned opts,
uint8_t *out, size_t *outlen) {
if (!(opts & (LOWER | DIACRITICS))) return false;
size_t o = 0;
for (size_t i = 0; i < len;) {
uint8_t c = in[i++];
uint32_t cp;
if (c <= 0x7f) {
if ((opts & LOWER) && (c >= 'A' && c <= 'Z')) {
cp = c | 0x20;
} else {
if ((opts & DIACRITICS) && (c == '^' || c == '`')) continue;
cp = c;
}
} else {
if (i >= len) return false;
cp = ((c & 0x1f) << 6) | (in[i++] & 0x3f);
if (opts & LOWER) cp = to_lower_slow(cp);
if (opts & DIACRITICS) { cp = strip_slow(cp); if (!cp) continue; }
}
if (cp <= 0x7f) out[o++] = (uint8_t)cp;
else { out[o++] = (uint8_t)((cp >> 6) | 0xc0); out[o++] = (uint8_t)((cp & 0x3f) | 0x80); }
}
out[o] = 0; *outlen = o; return true;
}
int main(void) {
uint8_t out[64]; size_t n;
fold((const uint8_t *)"Wilson", 6, LOWER, out, &n); printf("LOWER -> %s (expect wilson)\n", out);
fold((const uint8_t *)"Wilson", 6, DIACRITICS, out, &n); printf("DIACRITICS -> %s (expect Wilson)\n", out);
}
```

```c
// helpers.c
#include
uint32_t to_lower_slow(uint32_t cp) { return (cp >= 0xC0 && cp <= 0xDE && cp != 0xD7) ? cp + 0x20 : cp; }
uint32_t strip_slow(uint32_t cp) { if (cp >= 0xF9 && cp <= 0xFC) return 'u'; if (cp == 0x5E || cp == 0x60) return 0; return cp; }
```

`clang --target=powerpc64le-linux-gnu -O2 -mcpu=power8` (run under qemu-ppc64le):

```
LOWER -> Wilson (expect wilson)
DIACRITICS -> wilson (expect Wilson)
```

The original victim was libmongocrypt's `unicode_fold()`, where this silently
disabled case folding for MongoDB Queryable Encryption text search on ppc64le.

## Analysis

The existing safety check in `optimizeCompareInstr` scans backward from the first
use of the compare's result to `MI`, rejecting instructions that read or write CR0.
That has two gaps for this case:

1. The reader (`%6 = COPY $cr0gt`) sits *after* the first use of the compare, so
it is outside the scanned range regardless of other conditions.
2. The scan is skipped entirely when `noSub` is set, which is the case for every
32-bit compare on PPC64 (`CMPWI` with a sign-extended operand, `CMPLWI` with a
zero-extended one).

Note that only fixing (2) — running the existing backward scan even when `noSub` is
set — is not sufficient: I tried exactly that, and the MIR above is still converted
(machine verifier still fails, C reproducer still wrong), because of (1).

## Fix

Before converting `MI` to a record form, check that CR0 is dead immediately after
`MI` using `MachineBasicBlock::computeRegisterLiveness`, scanning to the end of the
block (the same cost profile as the existing scans in the function) and treating an
inconclusive result as live. Candidates that are already record-form instructions
do not introduce a new CR0 def and are unaffected.

Verified on main (d8217655d) and on release/19.x (19.1.7), PowerPC-only builds with
assertions on. On main, `test/CodeGen/PowerPC` has an identical pass/fail set with and
without the patch (the remaining failures in my environment are tests needing tools I
did not build), except that the new `cmp-opt-cr0-live.mir` fails on stock main and
passes with the patch. On 19.1.7:

- `cmp-opt-cr0-live.mir`: fails on stock `llc` (converted + verifier error), passes
with the fix.
- `check-llvm-codegen-powerpc`: 1872 passed, 8 expectedly failed, 7 unsupported; the
only 2 failures (`misched.ll`, `git_revision.ll`) need a native default target,
which the PowerPC-only build lacks — unrelated.
- The C reproducer and the original libmongocrypt `unicode_fold()` code, compiled
through the patched `llc` 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).

---

*Disclosure (per the LLVM developer policy on AI-assisted contributions): the initial
analysis, reproduction and drafting of this report were done with the assistance of an
AI coding tool; I have reproduced the failure and every verification result shown above
and am submitting this as the responsible author.*

Contributor guide

Open the contributing guide

Research direction

Start at PPCInstrInfo::optimizeCompareInstr and inspect its CR0 liveness checks, then run llc with the cmp-opt-cr0-live.mir reproducer and -verify-machineinstrs. The change is complete when a candidate that would clobber live CR0 is not converted, the MIR test passes, and the existing test/CodeGen/PowerPC suite remains consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.