[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