-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[VPlan] Implement printing VPIRMetadata. #165825
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
base: main
Are you sure you want to change the base?
Conversation
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
@llvm/pr-subscribers-vectorizers @llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesImplement and use debug printing for VPIRMetadata. Full diff: https://github.com/llvm/llvm-project/pull/165825.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 1f10058ab4a9a..79583ce34c74d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -968,6 +968,9 @@ class VPIRMetadata {
/// Intersect this VPIRMetada object with \p MD, keeping only metadata
/// nodes that are common to both.
void intersect(const VPIRMetadata &MD);
+
+ /// Print metadata with node IDs.
+ void print(raw_ostream &O, const Module &M) const;
};
/// This is a concrete Recipe that models a single VPlan-level instruction.
@@ -4268,6 +4271,11 @@ class VPlan {
/// Return the VPIRBasicBlock wrapping the header of the scalar loop.
VPIRBasicBlock *getScalarHeader() const { return ScalarHeader; }
+ /// Return the Module from the scalar header.
+ const Module &getModule() const {
+ return *ScalarHeader->getIRBasicBlock()->getModule();
+ }
+
/// Return an ArrayRef containing VPIRBasicBlocks wrapping the exit blocks of
/// the original scalar loop.
ArrayRef<VPIRBasicBlock *> getExitBlocks() const { return ExitBlocks; }
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index bde62dd6dd4bc..132bb50e92a7f 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -1443,6 +1443,7 @@ void VPInstruction::print(raw_ostream &O, const Twine &Indent,
printFlags(O);
printOperands(O, SlotTracker);
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
if (auto DL = getDebugLoc()) {
O << ", !dbg ";
@@ -1669,6 +1670,26 @@ void VPIRMetadata::intersect(const VPIRMetadata &Other) {
Metadata = std::move(MetadataIntersection);
}
+void VPIRMetadata::print(raw_ostream &O, const Module &M) const {
+ if (Metadata.empty())
+ return;
+
+ SmallVector<StringRef, 8> MDNames;
+ M.getContext().getMDKindNames(MDNames);
+
+ O << " (";
+ interleaveComma(Metadata, O, [&](const auto &KindNodePair) {
+ auto [Kind, Node] = KindNodePair;
+ assert(Kind != 0 && "Debug metadata should not be managed by VPIRMetadata");
+ assert(Kind < MDNames.size() && !MDNames[Kind].empty() &&
+ "Unexpected unnamed metadata kind");
+ O << "!" << MDNames[Kind] << " ";
+ Node->printAsOperand(O, &M);
+ });
+ O << ")";
+}
+
+
void VPWidenCallRecipe::execute(VPTransformState &State) {
assert(State.VF.isVector() && "not widening");
assert(Variant != nullptr && "Can't create vector function.");
@@ -1729,6 +1750,7 @@ void VPWidenCallRecipe::print(raw_ostream &O, const Twine &Indent,
Op->printAsOperand(O, SlotTracker);
});
O << ")";
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
O << " (using library function";
if (Variant->hasName())
@@ -1863,6 +1885,7 @@ void VPWidenIntrinsicRecipe::print(raw_ostream &O, const Twine &Indent,
Op->printAsOperand(O, SlotTracker);
});
O << ")";
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
}
#endif
@@ -2255,6 +2278,7 @@ void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
O << " = " << Instruction::getOpcodeName(Opcode);
printFlags(O);
printOperands(O, SlotTracker);
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
}
#endif
@@ -2336,6 +2360,7 @@ void VPWidenCastRecipe::print(raw_ostream &O, const Twine &Indent,
printFlags(O);
printOperands(O, SlotTracker);
O << " to " << *getResultType();
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
}
#endif
@@ -3618,6 +3643,7 @@ void VPWidenLoadRecipe::print(raw_ostream &O, const Twine &Indent,
printAsOperand(O, SlotTracker);
O << " = load ";
printOperands(O, SlotTracker);
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
}
#endif
@@ -3739,6 +3765,7 @@ void VPWidenStoreRecipe::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
O << Indent << "WIDEN store ";
printOperands(O, SlotTracker);
+ VPIRMetadata::print(O, getParent()->getPlan()->getModule());
}
#endif
|
bcd6751 to
368bacd
Compare
Implement and use debug printing for VPIRMetadata.
368bacd to
6cf1ed5
Compare
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.
LG, but it seems some recipes were missed, such as the interleave recipe.
Wonder if there's a way to catch/enforce all derived recipes to print their metadata; seems difficult by intercepting |
| /// Intersect this VPIRMetada object with \p MD, keeping only metadata | ||
| /// nodes that are common to both. | ||
| void intersect(const VPIRMetadata &MD); | ||
|
|
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.
Under
| #if !defined(NDEBUG) || defined(LLVM_ENABLE_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.
Done thanks
|
|
||
| /// Return true if there is any metadata to print. | ||
| bool empty() const { return Metadata.empty(); } |
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.
Redundant - print() checks anyhow if there is any metadata to print?
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.
This was used to check before printing; removed now, thanks
| /// Return the Module from the scalar header. | ||
| const Module &getModule() const { | ||
| return *ScalarHeader->getIRBasicBlock()->getModule(); | ||
| } | ||
|
|
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.
Have VPlan return Module only for VPIRMetadata to print itself (under DEBUG)?
Could (the Module of) VPSlotTracker's MST be used instead?
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.
I don't think so, as MST also may not be set. I update the code to return the module by reference, so the empty check can be removed before call print.
| if (Metadata.empty()) | ||
| return; |
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.
Empty checked anyhow.
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.
Yes, used to check at callsite before dereferencing Module. Updated now, only checked here.
Add a new pinrRecipe which handles printing the recipe without common info like debug info or metadata. Prepares to print them once, in ::print(), after/in combination with llvm#165825.
Implement CastInfo from VPRecipeBase to VPIRMetadata to support isa/dyn_Cast. This is similar to CastInfoVPPhiAccessors, supporting dyn_cast by down-casting to the concrete recipe types inheriting from VPIRMetadata. Can be used for more generalized VPIRMetadata printing following llvm#165825.
f1e7f7d to
02a8a9a
Compare
02a8a9a to
0dc9780
Compare
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.
LG, but it seems some recipes were missed, such as the interleave recipe.
Wonder if there's a way to catch/enforce all derived recipes to print their metadata; seems difficult by intercepting
VPValue::printAsOperand()due to multiple inheritance(?)
One option would be to change recipe's printing without common info like metadata to be called printRecipe, then have VPRecipeBase::print() call printRecipe and print common metadata (and others like debug info): #166244
We will also need to be able to check if a VPRecipeBase has VPIRMetadata, which can be done by implemeting CastInfo: #166245
LG, but it seems some recipes were missed, such as the interleave recipe.
Thanks, added some of the missing ones
|
|
||
| /// Return true if there is any metadata to print. | ||
| bool empty() const { return Metadata.empty(); } |
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.
This was used to check before printing; removed now, thanks
| /// Return the Module from the scalar header. | ||
| const Module &getModule() const { | ||
| return *ScalarHeader->getIRBasicBlock()->getModule(); | ||
| } | ||
|
|
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.
I don't think so, as MST also may not be set. I update the code to return the module by reference, so the empty check can be removed before call print.
| if (Metadata.empty()) | ||
| return; |
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.
Yes, used to check at callsite before dereferencing Module. Updated now, only checked here.
Implement and use debug printing for VPIRMetadata.