-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[DebugInfo][LoopVectorizer][NFC] Use unknown annotations for more instructions #170522
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
Conversation
…cations Some recent patches have added more non-annotated empty locations to the loop vectorizer, resulting in errors reported on the DebugLoc coverage tracking buildbot: https://lab.llvm.org/staging/#/builders/222/builds/1938 This patch adds "unknown" annotations in place of the empty locations, allowing the buildbot to ignore them for now.
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-vectorizers Author: Stephen Tozer (SLTozer) ChangesSome recent patches have added more non-annotated empty locations to the loop vectorizer, resulting in errors reported on the DebugLoc coverage tracking buildbot: This patch adds "unknown" annotations in place of the empty locations, allowing the buildbot to ignore them for now. Full diff: https://github.com/llvm/llvm-project/pull/170522.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 38024aa6897fc..6429ff8e5e1b9 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -967,7 +967,7 @@ static VPValue *optimizeLatchExitInductionUser(
VPValue *Step = WideIV->getStepValue();
Type *ScalarTy = TypeInfo.inferScalarType(WideIV);
if (ScalarTy->isIntegerTy())
- return B.createNaryOp(Instruction::Sub, {EndValue, Step}, {}, "ind.escape");
+ return B.createNaryOp(Instruction::Sub, {EndValue, Step}, DebugLoc::getUnknown(), "ind.escape");
if (ScalarTy->isPointerTy()) {
Type *StepTy = TypeInfo.inferScalarType(Step);
auto *Zero = Plan.getConstantInt(StepTy, 0);
@@ -3770,11 +3770,11 @@ void VPlanTransforms::handleUncountableEarlyExit(VPBasicBlock *EarlyExitingVPBB,
if (!IncomingFromEarlyExit->isLiveIn()) {
// Update the incoming value from the early exit.
VPValue *FirstActiveLane = EarlyExitB.createNaryOp(
- VPInstruction::FirstActiveLane, {CondToEarlyExit}, nullptr,
+ VPInstruction::FirstActiveLane, {CondToEarlyExit}, DebugLoc::getUnknown(),
"first.active.lane");
IncomingFromEarlyExit = EarlyExitB.createNaryOp(
VPInstruction::ExtractLane, {FirstActiveLane, IncomingFromEarlyExit},
- nullptr, "early.exit.value");
+ DebugLoc::getUnknown(), "early.exit.value");
ExitIRI->setOperand(EarlyExitIdx, IncomingFromEarlyExit);
}
}
@@ -4990,7 +4990,7 @@ void VPlanTransforms::updateScalarResumePhis(
"Cannot handle loops with uncountable early exits");
if (IsFOR)
ResumeFromVectorLoop = MiddleBuilder.createNaryOp(
- VPInstruction::ExtractLastElement, {ResumeFromVectorLoop}, {},
+ VPInstruction::ExtractLastElement, {ResumeFromVectorLoop}, DebugLoc::getUnknown(),
"vector.recur.extract");
ResumePhiR->setName(IsFOR ? "scalar.recur.init" : "bc.merge.rdx");
ResumePhiR->setOperand(0, ResumeFromVectorLoop);
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
artagnon
left a comment
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.
LGTM, with formatting fixed, thanks!
artagnon
left a comment
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.
Out of curiosity, when are we supposed to use DebugLoc::getCompilerGenerated()?
Thanks for the quick review! We use compiler generated annotations for instructions that are generated by the compiler but don't correspond to user code; there are some examples in global variable opt, loop unswitching, and tail recursion elimination right now, and also Clang applies it to any instructions it generates that weren't assigned a source location. It could also be appropriate here, the reason I've gone with "unknowns" is that I don't understand VPlan well enough to confidently select a different annotation. Further down the line I intend to work through the "unknown" annotations in the vectorizers to either rewrite them to more accurate annotations or to see if there's a way the instructions could receive valid source locations instead. |
Thanks for the explanation, and examples! You're right that VPlan is somewhat special, and the compiler-generated annotations might not work, since it's not exactly IR, but rather a separate overlay on the IR. We have to think about how to get accurate DebugLocs in VPlan some more. |
fhahn
left a comment
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.
LGTM, thanks
…tructions (llvm#170522) Some recent patches have added more non-annotated empty locations to the loop vectorizer, resulting in errors reported on the DebugLoc coverage tracking buildbot: https://lab.llvm.org/staging/#/builders/222/builds/1938 This patch adds "unknown" annotations in place of the empty locations, allowing the buildbot to ignore them for now.
Some recent patches have added more non-annotated empty locations to the loop vectorizer, resulting in errors reported on the DebugLoc coverage tracking buildbot:
https://lab.llvm.org/staging/#/builders/222/builds/1938
This patch adds "unknown" annotations in place of the empty locations, allowing the buildbot to ignore them for now.