[PATCH] D91027: [BasicAA] Generalize base offset modulus handling

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Unfortunately we are seeing mis-compiles caused by this patch. I don't think think the use of modulo arithmetic here is safe, unless the index computation is guaranteed to not overflow.

For the example below, this patch determines no-alias for `%gep.idx` and `%gep.6`. But if `%mul` overflows (e.g. `%idx == 52`), `%add == 6` and they are must alias. (https://godbolt.org/z/bWjz86K97)

I pushed a set of test cases in bcc8d80192f1 <https://reviews.llvm.org/rGbcc8d80192f17ee398db3effa36f4ad20844f8e1> and put up a candidate fix D99424 <https://reviews.llvm.org/D99424>

  ; %gep.idx and %gep.6 must-alias if %mul overflows (e.g. %idx == 52).
  define void @may_overflow_mul_add_i8([16 x i8]* %ptr, i8 %idx) {
    %mul = mul i8 %idx, 5
    %add = add i8 %mul, 2
    %gep.idx = getelementptr [16 x i8], [16 x i8]* %ptr, i32 0, i8 %add
    store i8 0, i8* %gep.idx, align 1
    %gep.6 = getelementptr [16 x i8], [16 x i8]* %ptr, i32 0, i32 6
    store i8 1, i8* %gep.6, align 1
    ret void
  }


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91027/new/

https://reviews.llvm.org/D91027