[llvm] Fix to generalize the canSkipClobberingStore (PR #174137)

Yunbo Ni via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 4 08:23:40 PST 2026


cardigan1008 wrote:

Hi, there is a related miscompilation test case:

```llvm
define i32 @test_gep_alias(ptr %p, ptr %p2, i32 %arg) {
entry:
  store i32 0, ptr %p
  store i32 %arg, ptr %p2
  %v = load i32, ptr %p
  ret i32 %v
}
```

After this patch, it's transformed into:

```llvm
define i32 @test_gep_alias(ptr %p, ptr %p2, i32 %arg) {
entry:
  store i32 0, ptr %p, align 4
  store i32 %arg, ptr %p2, align 4
  ret i32 0
}
```

The original transformation is:

```llvm
define i32 @test_gep_alias(ptr %p, ptr %p2, i32 %arg) {
entry:
  store i32 0, ptr %p, align 4
  store i32 %arg, ptr %p2, align 4
  %v = load i32, ptr %p, align 4
  ret i32 %v
}
```

Alive2 proof: https://alive2.llvm.org/ce/z/h5yBWm. 

> Note: This is a review assisted with a self-built AI agent. The reproducer was validated manually. Please let me know if anything is wrong.

The fix is fundamentally unsound because it assumes: If the store’s pointer operand has only one SSA use (`SIPtr->hasOneUse()`), then the underlying memory it points to is not accessed anywhere else.

This is not valid in LLVM IR:

- `hasOneUse()` is a property of an SSA value, not of the memory location.
- Multiple distinct SSA pointer values can refer to the same memory via aliasing.

So even if `%p2` is only used by one store instruction, it can still alias `%p`, and `%p` is used by the load. Therefore the store to `%p2` can still clobber the load from `%p`, and skipping it is incorrect.

**Consequence:** The analysis may ignore real clobbers, enabling GVN to forward stale/older values across intervening stores, producing wrong-code.

https://github.com/llvm/llvm-project/pull/174137


More information about the llvm-commits mailing list