[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