-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SLP] Normalize debug messages for newTreeEntry. #119514
[SLP] Normalize debug messages for newTreeEntry. #119514
Conversation
A debug message should follow after newTreeEntry. Make ExtractValueInst and ExtractElementInst use setOperand directly.
@llvm/pr-subscribers-llvm-transforms Author: Han-Kuan Chen (HanKuanChen) ChangesA debug message should follow after newTreeEntry. Full diff: https://github.com/llvm/llvm-project/pull/119514.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index a1d7515f031cfc..67d785016065ec 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -8388,7 +8388,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TreeEntry *TE =
newTreeEntry(VL, Bundle, S, UserTreeIdx, ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of PHINodes.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (PHINode).\n");
// Keeps the reordered operands to avoid code duplication.
PHIHandler Handler(*DT, PH, VL);
@@ -8417,13 +8417,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
}
// Insert new order with initial value 0, if it does not exist,
// otherwise return the iterator to the existing one.
- newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
- ReuseShuffleIndices, CurrentOrder);
+ TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
+ ReuseShuffleIndices, CurrentOrder);
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry "
+ "(ExtractValueInst/ExtractElementInst).\n");
// This is a special case, as it does not gather, but at the same time
// we are not extending buildTree_rec() towards the operands.
- ValueList Op0;
- Op0.assign(VL.size(), VL0->getOperand(0));
- VectorizableTree.back()->setOperand(0, Op0);
+ TE->setOperand(*this);
return;
}
case Instruction::InsertElement: {
@@ -8451,7 +8451,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
CurrentOrder.clear();
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
{}, CurrentOrder);
- LLVM_DEBUG(dbgs() << "SLP: added inserts bundle.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (InsertElementInst).\n");
TE->setOperand(*this);
buildTree_rec(TE->getOperand(1), Depth + 1, {TE, 1});
@@ -8471,21 +8471,25 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices, CurrentOrder, InterleaveFactor);
if (CurrentOrder.empty())
- LLVM_DEBUG(dbgs() << "SLP: added a vector of loads.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (LoadInst).\n");
else
- LLVM_DEBUG(dbgs() << "SLP: added a vector of jumbled loads.\n");
+ LLVM_DEBUG(dbgs()
+ << "SLP: added a new TreeEntry (jumbled LoadInst).\n");
break;
case TreeEntry::StridedVectorize:
// Vectorizing non-consecutive loads with `llvm.masked.gather`.
TE = newTreeEntry(VL, TreeEntry::StridedVectorize, Bundle, S,
UserTreeIdx, ReuseShuffleIndices, CurrentOrder);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of strided loads.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (strided LoadInst).\n");
break;
case TreeEntry::ScatterVectorize:
// Vectorizing non-consecutive loads with `llvm.masked.gather`.
TE = newTreeEntry(VL, TreeEntry::ScatterVectorize, Bundle, S,
UserTreeIdx, ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of non-consecutive loads.\n");
+ LLVM_DEBUG(
+ dbgs()
+ << "SLP: added a new TreeEntry (non-consecutive LoadInst).\n");
break;
case TreeEntry::CombinedVectorize:
case TreeEntry::NeedToGather:
@@ -8529,7 +8533,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
}
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of casts.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CastInst).\n");
TE->setOperand(*this);
for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
@@ -8556,7 +8560,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
CmpInst::Predicate P0 = cast<CmpInst>(VL0)->getPredicate();
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of compares.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CmpInst).\n");
ValueList Left, Right;
VLOperands Ops(VL, VL0, *this);
@@ -8626,7 +8630,9 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::Freeze: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of un/bin op.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry "
+ "(SelectInst/UnaryOperator/BinaryOperator/FreezeInst).\n");
TE->setOperand(*this, isa<BinaryOperator>(VL0) && isCommutative(VL0));
for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
@@ -8636,7 +8642,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::GetElementPtr: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of GEPs.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (GetElementPtrInst).\n");
SmallVector<ValueList, 2> Operands(2);
// Prepare the operand vector for pointer operands.
for (Value *V : VL) {
@@ -8694,12 +8700,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
fixupOrderingIndices(CurrentOrder);
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices, CurrentOrder);
- TE->setOperand(*this);
- buildTree_rec(TE->getOperand(0), Depth + 1, {TE, 0});
if (Consecutive)
- LLVM_DEBUG(dbgs() << "SLP: added a vector of stores.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (StoreInst).\n");
else
- LLVM_DEBUG(dbgs() << "SLP: added a vector of jumbled stores.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (jumbled StoreInst).\n");
+ TE->setOperand(*this);
+ buildTree_rec(TE->getOperand(0), Depth + 1, {TE, 0});
return;
}
case Instruction::Call: {
@@ -8710,6 +8717,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CallInst).\n");
TE->setOperand(*this, isCommutative(VL0));
for (unsigned I : seq<unsigned>(CI->arg_size())) {
// For scalar operands no need to create an entry since no need to
@@ -8723,7 +8731,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::ShuffleVector: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a ShuffleVector op.\n");
+ if (S.isAltShuffle()) {
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (alternate op).\n");
+ } else {
+ assert(SLPReVec && "Only supported by REVEC.");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (ShuffleVectorInst).\n");
+ }
// Reorder operands if reordering would enable vectorization.
auto *CI = dyn_cast<CmpInst>(VL0);
|
@llvm/pr-subscribers-vectorizers Author: Han-Kuan Chen (HanKuanChen) ChangesA debug message should follow after newTreeEntry. Full diff: https://github.com/llvm/llvm-project/pull/119514.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index a1d7515f031cfc..67d785016065ec 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -8388,7 +8388,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TreeEntry *TE =
newTreeEntry(VL, Bundle, S, UserTreeIdx, ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of PHINodes.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (PHINode).\n");
// Keeps the reordered operands to avoid code duplication.
PHIHandler Handler(*DT, PH, VL);
@@ -8417,13 +8417,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
}
// Insert new order with initial value 0, if it does not exist,
// otherwise return the iterator to the existing one.
- newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
- ReuseShuffleIndices, CurrentOrder);
+ TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
+ ReuseShuffleIndices, CurrentOrder);
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry "
+ "(ExtractValueInst/ExtractElementInst).\n");
// This is a special case, as it does not gather, but at the same time
// we are not extending buildTree_rec() towards the operands.
- ValueList Op0;
- Op0.assign(VL.size(), VL0->getOperand(0));
- VectorizableTree.back()->setOperand(0, Op0);
+ TE->setOperand(*this);
return;
}
case Instruction::InsertElement: {
@@ -8451,7 +8451,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
CurrentOrder.clear();
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
{}, CurrentOrder);
- LLVM_DEBUG(dbgs() << "SLP: added inserts bundle.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (InsertElementInst).\n");
TE->setOperand(*this);
buildTree_rec(TE->getOperand(1), Depth + 1, {TE, 1});
@@ -8471,21 +8471,25 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices, CurrentOrder, InterleaveFactor);
if (CurrentOrder.empty())
- LLVM_DEBUG(dbgs() << "SLP: added a vector of loads.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (LoadInst).\n");
else
- LLVM_DEBUG(dbgs() << "SLP: added a vector of jumbled loads.\n");
+ LLVM_DEBUG(dbgs()
+ << "SLP: added a new TreeEntry (jumbled LoadInst).\n");
break;
case TreeEntry::StridedVectorize:
// Vectorizing non-consecutive loads with `llvm.masked.gather`.
TE = newTreeEntry(VL, TreeEntry::StridedVectorize, Bundle, S,
UserTreeIdx, ReuseShuffleIndices, CurrentOrder);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of strided loads.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (strided LoadInst).\n");
break;
case TreeEntry::ScatterVectorize:
// Vectorizing non-consecutive loads with `llvm.masked.gather`.
TE = newTreeEntry(VL, TreeEntry::ScatterVectorize, Bundle, S,
UserTreeIdx, ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of non-consecutive loads.\n");
+ LLVM_DEBUG(
+ dbgs()
+ << "SLP: added a new TreeEntry (non-consecutive LoadInst).\n");
break;
case TreeEntry::CombinedVectorize:
case TreeEntry::NeedToGather:
@@ -8529,7 +8533,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
}
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of casts.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CastInst).\n");
TE->setOperand(*this);
for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
@@ -8556,7 +8560,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
CmpInst::Predicate P0 = cast<CmpInst>(VL0)->getPredicate();
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of compares.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CmpInst).\n");
ValueList Left, Right;
VLOperands Ops(VL, VL0, *this);
@@ -8626,7 +8630,9 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::Freeze: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of un/bin op.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry "
+ "(SelectInst/UnaryOperator/BinaryOperator/FreezeInst).\n");
TE->setOperand(*this, isa<BinaryOperator>(VL0) && isCommutative(VL0));
for (unsigned I : seq<unsigned>(VL0->getNumOperands()))
@@ -8636,7 +8642,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::GetElementPtr: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a vector of GEPs.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (GetElementPtrInst).\n");
SmallVector<ValueList, 2> Operands(2);
// Prepare the operand vector for pointer operands.
for (Value *V : VL) {
@@ -8694,12 +8700,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
fixupOrderingIndices(CurrentOrder);
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices, CurrentOrder);
- TE->setOperand(*this);
- buildTree_rec(TE->getOperand(0), Depth + 1, {TE, 0});
if (Consecutive)
- LLVM_DEBUG(dbgs() << "SLP: added a vector of stores.\n");
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (StoreInst).\n");
else
- LLVM_DEBUG(dbgs() << "SLP: added a vector of jumbled stores.\n");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (jumbled StoreInst).\n");
+ TE->setOperand(*this);
+ buildTree_rec(TE->getOperand(0), Depth + 1, {TE, 0});
return;
}
case Instruction::Call: {
@@ -8710,6 +8717,7 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (CallInst).\n");
TE->setOperand(*this, isCommutative(VL0));
for (unsigned I : seq<unsigned>(CI->arg_size())) {
// For scalar operands no need to create an entry since no need to
@@ -8723,7 +8731,13 @@ void BoUpSLP::buildTree_rec(ArrayRef<Value *> VL, unsigned Depth,
case Instruction::ShuffleVector: {
TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
ReuseShuffleIndices);
- LLVM_DEBUG(dbgs() << "SLP: added a ShuffleVector op.\n");
+ if (S.isAltShuffle()) {
+ LLVM_DEBUG(dbgs() << "SLP: added a new TreeEntry (alternate op).\n");
+ } else {
+ assert(SLPReVec && "Only supported by REVEC.");
+ LLVM_DEBUG(
+ dbgs() << "SLP: added a new TreeEntry (ShuffleVectorInst).\n");
+ }
// Reorder operands if reordering would enable vectorization.
auto *CI = dyn_cast<CmpInst>(VL0);
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be useful to dump the list of scalars, if possible, helps in many cases
how about dump |
Sounds good |
@@ -3671,6 +3671,7 @@ class BoUpSLP { | |||
|
|||
if (UserTreeIdx.UserTE) | |||
Last->UserTreeIndices.push_back(UserTreeIdx); | |||
Last->dump(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's a bad solution. 1. It first be emitted only in debug builds and only if -debug is provided. 2. Better to move it to each LLVM_DEBUG, where the messages for entries are printed
@@ -3671,7 +3671,7 @@ class BoUpSLP { | |||
|
|||
if (UserTreeIdx.UserTE) | |||
Last->UserTreeIndices.push_back(UserTreeIdx); | |||
Last->dump(); | |||
LLVM_DEBUG(Last->dump()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, make it part of debug messages, do not dump it here
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/9807 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/10836 Here is the relevant piece of the build log for the reference
|
A debug message should follow after newTreeEntry.
Make ExtractValueInst and ExtractElementInst use setOperand directly.