| Issue |
182731
|
| Summary |
[MachineCopyPropagation] Incorrect deletion of required physical-register copy when successor live-ins are stale
|
| Labels |
|
| Assignees |
|
| Reporter |
Sightem
|
`MachineCopyPropagation` (MCP) can incorrectly delete a physical register copy at block exit when it infers deadness solely from successor live-in metadata. If that metadata is stale or incomplete, MCP removes a copy that is still read in a successor before any redefinition, producing incorrect code at runtime.
This is a **codegen correctness bug**. It was discovered through a real miscompile on an out-of-tree target (eZ80), reduced to an in-tree x86 unit-test reproducer and confirmed present on trunk.
## How this was discovered
The bug surfaced while upgrading our [eZ80 backend](https://github.com/CE-Programming/llvm-project/tree/z80-19.1.0) from LLVM 18 to LLVM 19. A LaTeX math rendering library (`[libtexce](https://github.com/Sightem/libtexce) began producing visibly corrupt output for matrix expressions. The corruption was deterministic and reproducible across two independent test cases.
Using `opt-bisect-limit`, the first bad pass was identified as **Machine Copy Propagation** on [draw_node](https://github.com/Sightem/libtexce/blob/1db04f7b27b57267e44c5fdde7f0e5853a8459b1/src/tex/tex_draw.c#L940) (pass index 2718; index 2717 produced correct output). The codegen delta at that boundary was a single deleted instruction.
## The eZ80 manifestation
On eZ80, the deleted instruction was `ld a, l` which is a register to register copy that loads the value of `L` into `A`. This matters because of how eZ80 comparison instructions work:
- `cp a, l` compares `A` with `L` by computing `A - L` and setting flags only. It does not modify `A`
- `ld a, l` copies the value of `L` into `A`. This is a true data move
In correct codegen, the sequence was:
```asm
cp a, l ; compare A with L, set flags (A unchanged)
ld a, l ; copy L into A (A now holds the loop index)
...
inc a ; increment the loop index
```
After MCP incorrectly deleted the `ld a, l`:
```asm
cp a, l ; compare A with L, set flags (A unchanged)
...
inc a ; increment stale A, wrong value
```
Because `cp` does not update `A`, the subsequent `inc a` operated on a stale value instead of the current loop index (`search_idx`). This corrupted the matrix cell iteration logic in `draw_matrix`, causing the visible rendering failures.
MCP's debug output confirmed the reasoning:
```
MCP: Removing copy due to no live-out succ: $a = COPY killed $l
```
The copy was semantically required. the successor block read `$a` before redefining it, but MCP concluded it was dead based on (stale) successor live-in information.
## Root cause
In the dead copy cleanup path of `ForwardCopyPropagateBlock`, MCP iterates over `MaybeDeadCopies` and deletes them when `TracksLiveness` is true, on the assumption that successor live-ins accurately reflect what is actually live across the edge.
If successor live-in metadata is stale or incomplete for a physical register, MCP sees no live-out demand and deletes the copy. There is no fallback check for whether the register is actually used in a successor before being redefined
## Reproducer
The accompanying unit test (`MachineCopyPropagationTest.KeepsCopyWhenSuccessorUsesRegBeforeDef`) demonstrates this on x86. It constructs a two block function where:
1. The entry block contains `$eax = COPY killed $ecx` and jumps to a successor
2. The successor uses `$eax` (via `ADD32rr`) before any redefinition
3. Successor live-ins are then cleared to simulate stale metadata
Without the fix, MCP deletes the copy and `countCopies(EntryMBB)` drops to 0. With the fix, the copy is preserved.
## Proposed fix
A small conservative guard before the dead-copy deletion: scan each successor block linearly, skipping PHIs. If the destination register is read before any def/modification, the copy is kept. If the first relevant event is a def, that successor is safe.
This only applies to physical-register copies in the maybe-dead deletion path and does not affect virtual register handling or other MCP transforms.
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
