[VPlan] Use Unknown instead of empty location in VPlanTransforms #157702
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The default values for DebugLocs in LoopVectorizer/VPlan were recently updated from empty DebugLocs to DebugLoc::getUnknown, as part of the Debug Loc Coverage Tracking work. However, there are some cases where we also pass an explicit empty DebugLoc, in some cases intentionally, and in others as a filler argument. This patch updates all of these to `getUnknown` for now, until a more principled approach to debug location propagation in the vectorizers is implemented.
SLTozer
requested review from
artagnon,
fhahn,
jmorse,
OCHyams and
CarlosAlbertoEnciso
|
@llvm/pr-subscribers-llvm-transforms Author: Stephen Tozer (SLTozer) ChangesThe default values for DebugLocs in LoopVectorizer/VPlan were recently updated from empty DebugLocs to DebugLoc::getUnknown, as part of the DebugLoc Coverage Tracking work. However, there are some cases where we also pass an explicit empty DebugLoc, in many cases as a filler argument. This patch updates all of these to This change is NFC outside of DebugLoc coverage tracking builds. Full diff: https://github.com/llvm/llvm-project/pull/157702.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 7de94717f56e5..714150bd98066 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -349,7 +349,7 @@ static VPRegionBlock *createReplicateRegion(VPReplicateRecipe *PredRecipe,
auto *BlockInMask = PredRecipe->getMask();
auto *MaskDef = BlockInMask->getDefiningRecipe();
auto *BOMRecipe = new VPBranchOnMaskRecipe(
- BlockInMask, MaskDef ? MaskDef->getDebugLoc() : DebugLoc());
+ BlockInMask, MaskDef ? MaskDef->getDebugLoc() : DebugLoc::getUnknown());
auto *Entry =
Plan.createVPBasicBlock(Twine(RegionName) + ".entry", BOMRecipe);
@@ -864,8 +864,8 @@ static VPValue *optimizeLatchExitInductionUser(
Type *StepTy = TypeInfo.inferScalarType(Step);
auto *Zero = Plan.getOrAddLiveIn(ConstantInt::get(StepTy, 0));
return B.createPtrAdd(EndValue,
- B.createNaryOp(Instruction::Sub, {Zero, Step}), {},
- "ind.escape");
+ B.createNaryOp(Instruction::Sub, {Zero, Step}),
+ DebugLoc::getUnknown(), "ind.escape");
}
if (ScalarTy->isFloatingPointTy()) {
const auto &ID = WideIV->getInductionDescriptor();
@@ -2307,7 +2307,8 @@ static VPActiveLaneMaskPHIRecipe *addVPLaneMaskPhiAndUpdateExitBranch(
// Now create the ActiveLaneMaskPhi recipe in the main loop using the
// preheader ActiveLaneMask instruction.
- auto *LaneMaskPhi = new VPActiveLaneMaskPHIRecipe(EntryALM, DebugLoc());
+ auto *LaneMaskPhi =
+ new VPActiveLaneMaskPHIRecipe(EntryALM, DebugLoc::getUnknown());
LaneMaskPhi->insertAfter(CanonicalIVPHI);
// Create the active lane mask for the next iteration of the loop before the
@@ -2534,11 +2535,11 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
VPBuilder Builder(LoopRegion->getPreheaderVPBB());
MaxEVL = Builder.createScalarZExtOrTrunc(
MaxEVL, Type::getInt32Ty(Plan.getContext()),
- TypeInfo.inferScalarType(MaxEVL), DebugLoc());
+ TypeInfo.inferScalarType(MaxEVL), DebugLoc::getUnknown());
Builder.setInsertPoint(Header, Header->getFirstNonPhi());
- VPValue *PrevEVL =
- Builder.createScalarPhi({MaxEVL, &EVL}, DebugLoc(), "prev.evl");
+ VPValue *PrevEVL = Builder.createScalarPhi(
+ {MaxEVL, &EVL}, DebugLoc::getUnknown(), "prev.evl");
for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
vp_depth_first_deep(Plan.getVectorLoopRegion()->getEntry()))) {
@@ -2668,7 +2669,7 @@ void VPlanTransforms::addExplicitVectorLength(
VPValue *StartV = CanonicalIVPHI->getStartValue();
// Create the ExplicitVectorLengthPhi recipe in the main loop.
- auto *EVLPhi = new VPEVLBasedIVPHIRecipe(StartV, DebugLoc());
+ auto *EVLPhi = new VPEVLBasedIVPHIRecipe(StartV, DebugLoc::getUnknown());
EVLPhi->insertAfter(CanonicalIVPHI);
VPBuilder Builder(Header, Header->getFirstNonPhi());
// Create the AVL (application vector length), starting from TC -> 0 in steps
@@ -2682,10 +2683,11 @@ void VPlanTransforms::addExplicitVectorLength(
VPValue *AVLSafe =
Plan.getOrAddLiveIn(ConstantInt::get(CanIVTy, *MaxSafeElements));
VPValue *Cmp = Builder.createICmp(ICmpInst::ICMP_ULT, AVL, AVLSafe);
- AVL = Builder.createSelect(Cmp, AVL, AVLSafe, DebugLoc(), "safe_avl");
+ AVL = Builder.createSelect(Cmp, AVL, AVLSafe, DebugLoc::getUnknown(),
+ "safe_avl");
}
auto *VPEVL = Builder.createNaryOp(VPInstruction::ExplicitVectorLength, AVL,
- DebugLoc());
+ DebugLoc::getUnknown());
auto *CanonicalIVIncrement =
cast<VPInstruction>(CanonicalIVPHI->getBackedgeValue());
@@ -3112,8 +3114,8 @@ expandVPWidenIntOrFpInduction(VPWidenIntOrFpInductionRecipe *WidenIVR,
VPValue *SplatStep = Builder.createNaryOp(VPInstruction::Broadcast, Step);
Init = Builder.createNaryOp(MulOp, {Init, SplatStep}, Flags);
- Init =
- Builder.createNaryOp(AddOp, {SplatStart, Init}, Flags, {}, "induction");
+ Init = Builder.createNaryOp(AddOp, {SplatStart, Init}, Flags,
+ DebugLoc::getUnknown(), "induction");
// Create the widened phi of the vector IV.
auto *WidePHI = new VPWidenPHIRecipe(WidenIVR->getPHINode(), nullptr,
|
|
@llvm/pr-subscribers-vectorizers Author: Stephen Tozer (SLTozer) ChangesThe default values for DebugLocs in LoopVectorizer/VPlan were recently updated from empty DebugLocs to DebugLoc::getUnknown, as part of the DebugLoc Coverage Tracking work. However, there are some cases where we also pass an explicit empty DebugLoc, in many cases as a filler argument. This patch updates all of these to This change is NFC outside of DebugLoc coverage tracking builds. Full diff: https://github.com/llvm/llvm-project/pull/157702.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 7de94717f56e5..714150bd98066 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -349,7 +349,7 @@ static VPRegionBlock *createReplicateRegion(VPReplicateRecipe *PredRecipe,
auto *BlockInMask = PredRecipe->getMask();
auto *MaskDef = BlockInMask->getDefiningRecipe();
auto *BOMRecipe = new VPBranchOnMaskRecipe(
- BlockInMask, MaskDef ? MaskDef->getDebugLoc() : DebugLoc());
+ BlockInMask, MaskDef ? MaskDef->getDebugLoc() : DebugLoc::getUnknown());
auto *Entry =
Plan.createVPBasicBlock(Twine(RegionName) + ".entry", BOMRecipe);
@@ -864,8 +864,8 @@ static VPValue *optimizeLatchExitInductionUser(
Type *StepTy = TypeInfo.inferScalarType(Step);
auto *Zero = Plan.getOrAddLiveIn(ConstantInt::get(StepTy, 0));
return B.createPtrAdd(EndValue,
- B.createNaryOp(Instruction::Sub, {Zero, Step}), {},
- "ind.escape");
+ B.createNaryOp(Instruction::Sub, {Zero, Step}),
+ DebugLoc::getUnknown(), "ind.escape");
}
if (ScalarTy->isFloatingPointTy()) {
const auto &ID = WideIV->getInductionDescriptor();
@@ -2307,7 +2307,8 @@ static VPActiveLaneMaskPHIRecipe *addVPLaneMaskPhiAndUpdateExitBranch(
// Now create the ActiveLaneMaskPhi recipe in the main loop using the
// preheader ActiveLaneMask instruction.
- auto *LaneMaskPhi = new VPActiveLaneMaskPHIRecipe(EntryALM, DebugLoc());
+ auto *LaneMaskPhi =
+ new VPActiveLaneMaskPHIRecipe(EntryALM, DebugLoc::getUnknown());
LaneMaskPhi->insertAfter(CanonicalIVPHI);
// Create the active lane mask for the next iteration of the loop before the
@@ -2534,11 +2535,11 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
VPBuilder Builder(LoopRegion->getPreheaderVPBB());
MaxEVL = Builder.createScalarZExtOrTrunc(
MaxEVL, Type::getInt32Ty(Plan.getContext()),
- TypeInfo.inferScalarType(MaxEVL), DebugLoc());
+ TypeInfo.inferScalarType(MaxEVL), DebugLoc::getUnknown());
Builder.setInsertPoint(Header, Header->getFirstNonPhi());
- VPValue *PrevEVL =
- Builder.createScalarPhi({MaxEVL, &EVL}, DebugLoc(), "prev.evl");
+ VPValue *PrevEVL = Builder.createScalarPhi(
+ {MaxEVL, &EVL}, DebugLoc::getUnknown(), "prev.evl");
for (VPBasicBlock *VPBB : VPBlockUtils::blocksOnly<VPBasicBlock>(
vp_depth_first_deep(Plan.getVectorLoopRegion()->getEntry()))) {
@@ -2668,7 +2669,7 @@ void VPlanTransforms::addExplicitVectorLength(
VPValue *StartV = CanonicalIVPHI->getStartValue();
// Create the ExplicitVectorLengthPhi recipe in the main loop.
- auto *EVLPhi = new VPEVLBasedIVPHIRecipe(StartV, DebugLoc());
+ auto *EVLPhi = new VPEVLBasedIVPHIRecipe(StartV, DebugLoc::getUnknown());
EVLPhi->insertAfter(CanonicalIVPHI);
VPBuilder Builder(Header, Header->getFirstNonPhi());
// Create the AVL (application vector length), starting from TC -> 0 in steps
@@ -2682,10 +2683,11 @@ void VPlanTransforms::addExplicitVectorLength(
VPValue *AVLSafe =
Plan.getOrAddLiveIn(ConstantInt::get(CanIVTy, *MaxSafeElements));
VPValue *Cmp = Builder.createICmp(ICmpInst::ICMP_ULT, AVL, AVLSafe);
- AVL = Builder.createSelect(Cmp, AVL, AVLSafe, DebugLoc(), "safe_avl");
+ AVL = Builder.createSelect(Cmp, AVL, AVLSafe, DebugLoc::getUnknown(),
+ "safe_avl");
}
auto *VPEVL = Builder.createNaryOp(VPInstruction::ExplicitVectorLength, AVL,
- DebugLoc());
+ DebugLoc::getUnknown());
auto *CanonicalIVIncrement =
cast<VPInstruction>(CanonicalIVPHI->getBackedgeValue());
@@ -3112,8 +3114,8 @@ expandVPWidenIntOrFpInduction(VPWidenIntOrFpInductionRecipe *WidenIVR,
VPValue *SplatStep = Builder.createNaryOp(VPInstruction::Broadcast, Step);
Init = Builder.createNaryOp(MulOp, {Init, SplatStep}, Flags);
- Init =
- Builder.createNaryOp(AddOp, {SplatStart, Init}, Flags, {}, "induction");
+ Init = Builder.createNaryOp(AddOp, {SplatStart, Init}, Flags,
+ DebugLoc::getUnknown(), "induction");
// Create the widened phi of the vector IV.
auto *WidePHI = new VPWidenPHIRecipe(WidenIVR->getPHINode(), nullptr,
|
|
@SLTozer is there any way to avoid/get notified when new uses of the default constructor are added? would be nice if we could avoid backsliding |
We don't have an automated notification at the moment, but the buildbot here is now green: https://lab.llvm.org/staging/#/builders/222 Any new unannotated locations should cause a failure on the buildbot (if they occur on the buildbot, which is running an |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The default values for DebugLocs in LoopVectorizer/VPlan were recently updated from empty DebugLocs to DebugLoc::getUnknown, as part of the DebugLoc Coverage Tracking work. However, there are some cases where we also pass an explicit empty DebugLoc, in many cases as a filler argument. This patch updates all of these to
getUnknownfor now, until either valid locations or a suitable categorization can be assigned to each instead.This change is NFC outside of DebugLoc coverage tracking builds.