[PATCH] D110977: [BasicAA] Model implicit trunc of GEP indices

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.
Herald added a subscriber: jeroen.dobbelaere.

Quick scan only.  I don't have a lot of context on this code.

Approach wise, it would seem lower risk to model explicit truncates before adding the implicit trunc on gep operands.  (i.e. easier to write test cases, less likelihood of wide spread miscompiles if we got it slightly wrong, less interaction w/e.g. "pointer size" piece)  Can I ask why you approached it this was instead?



================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:269
+/// Represents zext(sext(trunc(V))).
 struct ExtendedValue {
   const Value *V;
----------------
Naming wise, "Extended" no longer seems correct.  Can you change this to something like CastedValue?  (In a separate NFC is encouraged.)


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:292
                         NewV->getType()->getPrimitiveSizeInBits();
+    if (ExtendBy <= TruncBits)
+      return ExtendedValue(NewV, ZExtBits, SExtBits, TruncBits - ExtendBy);
----------------
I don't believe this is correct.  What if we trunced off non-zero bits and are no zero extending?


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

https://reviews.llvm.org/D110977