[PATCH] D28907: [SLP] Fix for PR30787: Failure to beneficially vectorize 'copyable' elements in integer binary ops.

Alexey Bataev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 19 08:13:51 PST 2017

ABataev added inline comments.

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3244
         isJumbled = true;
-        LI = cast<LoadInst>(E->Scalars[0]);
-      } else {
-        LI = cast<LoadInst>(VL0);
-      }
+      LI = cast<LoadInst>(E->Scalars[0]);
dtemirbulatov wrote:
> ABataev wrote:
> > dtemirbulatov wrote:
> > > We used E->VL0 here before, which is not correct because it could point to a wrong element.
> > Why could it point to a wrong element? As I understand, it should always point to Scalars[0] for LoadInst.
> No, it is in order as it was discovered. I will add a testcase for the issue.
I still don't understand what's the problem here. Why we can't use `VL0 ` in both cases?

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3707-3708
+  // Check that all non-alternative operations dominate at least
+  // one real vector operation.
+  if (S.IsNonAlt) {
ABataev wrote:
> What is the `real vector operation`? And why do you need this check? You mean that it is allowed to have sequence `load, add`, but not `add ,load`? Why?
Seems to me, what you need to fix is `setInsertPointAfterBundle` function rather than addinng|checking domination here.


More information about the llvm-commits mailing list