[PATCH] D88669: [ValueTracking] Add tracking of the alignment assume bundle

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Some nits you should address, otherwise LGTM.

In D88669#2330433 <https://reviews.llvm.org/D88669#2330433>, @scui wrote:

> Rewrite the code using getKnowledgeValidInContext. The code is much cleaner than the previous version, but the result computed is less accurate if the size of the alignment bundle is three. I'll leave the improvement for the future, together with the improvement of value tracking of GEP (an example is that, with the future improvement, we can fold away the branch "br i1 %3, ...") in f2 of the added case assume-align.ll).

FWIW, we should either not encode the offset but use a GEP, or make `getKnowledgeValidInContext` aware. Either should be a follow up.



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:681
+    }
+  }
+
----------------
I don't think the parenthesis part in the comment should be here.


================
Comment at: llvm/test/Transforms/InstCombine/assume-align.ll:48
+
+define void @f2(i8* %a) {
+; CHECK-LABEL: @f2(
----------------
Instead of the comment above, add a TODO/FIXME here describing the two potential solutions to this problem.


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

https://reviews.llvm.org/D88669