[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
Thu Apr 12 11:57:33 PDT 2018
ABataev added a comment.
Also, your code seems not quite formatted. Please, use clang-format on your changes to format it properly.
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:433
+ cast<Instruction>(VL[NewOpcodePos])))
+ Res.NeedAnalysis = true;
+ else
----------------
Enclose it into braces
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:435
+ else
+ return {};
}
----------------
This too
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:474
unsigned AltOpcode = getAltOpcode(Opcode);
+ bool IsNonAlt = false;
+ for (Value *V : VL) {
----------------
```
bool IsNonAlt = llvm::one_of(VL, [Opcode, AltOpcode](Value *V) {return isa<Instruction>(V) && !sameOpcodeOrAlt(Opcode, AltOpcode, cast<Instruction>(V)->getOpcode());});
```
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:1616
+ if (SameOrAlt <= (VL.size() / 2)) {
+ newTreeEntry(VL, false, UserTreeIdx, S);
+ return;
----------------
Add the debug message here
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3112
+ int MaxDist = 0;
+ while (Bundle) {
+ int Dist = std::distance(BB->begin(), Bundle->Inst->getIterator());
----------------
Why you can't put bundles in the list in the right order: from the very first instruction to the very last?
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:3938-3945
+ for (User *U : Scalar->users()) {
+ if (!getTreeEntry(U) && getTreeEntry(U, Scalar) == Entry) {
+ Skip = true;
+ break;
+ }
+ }
+ if (Skip)
----------------
Better to do it this way:
```
if (llvm::any_of(Scalar->users(), [this, Entry, Scalar](User *U){return !getTreeEntry(U) && getTreeEntry(U, Scalar) == Entry;}))
continue;
```
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4481
+ // regular opertions in order to stay within a bundle boundaries.
+ for (Instruction *pickedInst : NonAlters)
if (LastScheduledInst->getNextNode() != pickedInst) {
----------------
`pickedInst`->`PickedInst`
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4486
pickedInst);
+ Value *V = dyn_cast<Value>(pickedInst);
+ ScheduleData *SD = BS->getScheduleData(V);
----------------
Remove this, it is not needed
================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4500-4502
+ auto *OpValue = dyn_cast<Value>(I);
+ if (!OpValue)
+ continue;
----------------
Remove it, does not do anything
https://reviews.llvm.org/D28907
More information about the llvm-commits
mailing list