[PATCH] D153752: [InstSimplify] Fold icmp comparing GEPs with local allocation

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

This mostly looks good to me.

As implemented, there was a potential issue here where you would get an assertion failure due to different bit widths if certain address space casts were involved. I've opted to make that impossible in https://github.com/llvm/llvm-project/commit/793eb0c0e4ffadc1cfebcda0c0cb1bcc2311def2 rather than have you introduce extra extensions here.



================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:2799
+        APInt Dist;
+        Dist = LHSOffset - RHSOffset;
+        if (Dist.isNonNegative() ? Dist.ult(LHSSize) : (-Dist).ult(RHSSize))
----------------
Combine declaration and initialization.


================
Comment at: llvm/test/Transforms/InstSimplify/cmp-alloca-offsets.ll:173
   ret i1 %res
 }
 
----------------
We should have some additional test cases that work on differently-sized allocas, to show that the check is asymmetric. E.g. allocas with size 2 and 4 with effective diffs 2 and -2.


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

https://reviews.llvm.org/D153752