[PATCH] D94713: Do not traverse ConstantData use-list in SLPVectorizer

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:989
         Value *V = Values[Idx].first;
+        if (isa<ConstantData>(V)) {
+          // Walking the use list of a Constant (particularly, ConstantData)
----------------
dexonsmith wrote:
> willir wrote:
> > nikic wrote:
> > > Why does this check for ConstantData in particular?
> > Hi, thanks for the review.
> > 
> > Instances of `ConstantData` are unique for the entire an `LLVMContext`, so its uses can be spread across multiple functions and modules.
> > This commit is a part of a bigger project to remove use-list from `ConstantData`.
> > 
> > Here are some links for more info about it:  
> > https://bugs.llvm.org/show_bug.cgi?id=30513  
> > https://lists.llvm.org/pipermail/llvm-dev/2016-September/105157.html
> It's possible that rejecting all Constants makes sense here, depending on whether this code needs to walk through GlobalValue-derived constants. I suspect it doesn't and we can just do Constant.
That sounds right to me - this pass is only concerned with "basic" integer/FP constants. For example, we have allConstant() around line 219:
  /// \returns True if all of the values in \p VL are constants (but not
  /// globals/constant expressions).

So I think it's safe to ignore any `Constant` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94713