[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
Mon Dec 18 08:47:38 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:
> 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.
================
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) {
----------------
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?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3729
+ }
+ // Check inner vector dependencies
+ for (Instruction *I1 : Alternatives)
----------------
dtemirbulatov wrote:
> I have observed some syntectic testcases where we could have cycle dependencies formed, I am sure without this check we could hit the issue somehow.
I rather doubt that this is required. Actually, you're not vectorizing the instruction itself, but the alternative pseudo-operation.
================
Comment at: test/Transforms/SLPVectorizer/X86/pr35497.ll:1
+; RUN: opt -slp-vectorizer -S -mtriple=x86_64-unknown-linux-gnu < %s
+
----------------
Where are the real checks?
https://reviews.llvm.org/D28907
More information about the llvm-commits
mailing list