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

[Shimin Cui via Phabricator via llvm-commits]

scui added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:799
+        return;
+      }
+
----------------
jdoerfert wrote:
> You don't want to look for the bundle yourself. Use
> ```
>   SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::Align};
>   if (RetainedKnowledge RK = getKnowledgeValidInContext(V, AttrKinds, CtxI, DT, AC)) {
>     ...
>   }
> ```
> instead to account for all the interesting cases, e.g., multiple `align` bundles. Also, use braces for non trivial conditionals. I don't think the return is what you want here, other deductions might still apply. Also, I think we have alignment bundle -> Align information in the `AlignmentFromAssumptions.cpp` pass. We should create a helper function so this is less magic.
> You don't want to look for the bundle yourself. Use
> ```
>   SmallVector<Attribute::AttrKind, 2> AttrKinds{Attribute::Align};
>   if (RetainedKnowledge RK = getKnowledgeValidInContext(V, AttrKinds, CtxI, DT, AC)) {
>     ...
>   }
> ```
> instead to account for all the interesting cases, e.g., multiple `align` bundles. Also, use braces for non trivial conditionals. I don't think the return is what you want here, other deductions might still apply. Also, I think we have alignment bundle -> Align information in the `AlignmentFromAssumptions.cpp` pass. We should create a helper function so this is less magic.

Thanks for the comments and suggestions. I have rewrite the code using getKnowledgeValidInContext. Please take a look. Thanks


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

https://reviews.llvm.org/D88669