[clang] [llvm] [InstCombine] Infer nuw for gep inbounds from base of object (PR #119225)
Florian Mayer via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 19 07:07:26 PST 2024
fmayer wrote:
I was working on this miscompile. I cannot give a reproducer right now, but the following observations:
The generated IR is clearly incorrect (the `inbounds nuw`), of the form:
```
%x = alloca [12 x i32], align 16
[...]
%59 = getelementptr inbounds nuw i8, ptr %x, i64 -4
%wide.masked.load.2 = call <4 x i32> @llvm.masked.load.v4i32.p0(ptr nonnull %59, i32 4, <4 x i1> <i1 false, i1 true, i1 true, i1 true>, <4 x i32> poison)
```
This is some interaction between this CL and these two InstCombines:
* https://github.com/llvm/llvm-project/blob/5a3f1acad7e8ce0e8cb90165794dce71f4b80bcd/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp#L2495
* https://github.com/llvm/llvm-project/blob/5a3f1acad7e8ce0e8cb90165794dce71f4b80bcd/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp#L2949
If I disable any of the three, the error goes away. The observation is that with the two above, we get an `inbounds` getelementptr that is _NOT_ inbounds (`-4`), that then gets masked vector read to correct for that.
The comment in the first link implies that it maybe should be removing `inbounds`:
```
// Even if the total offset is inbounds, we may end up representing it
// by first performing a larger negative offset, and then a smaller
// positive one. The large negative offset might go out of bounds. Only
// preserve inbounds if all signs are the same.
```
but it then goes and only removes the wrap flags:
```
if (Idx.isNonNegative() != ConstIndices[0].isNonNegative())
NW = NW.withoutNoUnsignedSignedWrap();
if (!Idx.isNonNegative())
NW = NW.withoutNoUnsignedWrap();
```
If I change this to
```
if (Idx.isNonNegative() != ConstIndices[0].isNonNegative())
NW = NW.withoutNoUnsignedSignedWrap().withoutInBounds();
if (!Idx.isNonNegative())
NW = NW.withoutNoUnsignedWrap().withoutInBounds();
```
the error also goes away.
If I disable the second link (i8 canonicalization) the access is then `-1` (makes sense) but *NOT* marked as `inbounds`.
I don't feel like this is a sanitizer issue, it seems like MSan might just have gotten unlucky.
https://github.com/llvm/llvm-project/pull/119225
More information about the llvm-commits
mailing list