-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[LV][NFC] Rename getNumUsers to getNumUses #155397
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
|
@llvm/pr-subscribers-llvm-transforms Author: David Sherwood (david-arm) ChangesThe Users list in VPValue is really just a Use list because the same User can appear several times in the list, reflecting the fact the VPValue can be used for more than one operand of a recipe. In such a scenario there is really just one user with multiple uses of a given VPValue. See the documentation for the IR routine Value::hasOneUser(): I've renamed this function, along with addUser and removeUser to mirror the normal IR terminology. Full diff: https://github.com/llvm/llvm-project/pull/155397.diff 9 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index fff4ad752e180..7aa6706fc0bb7 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -4290,7 +4290,7 @@ VectorizationFactor LoopVectorizationPlanner::selectVectorizationFactor() {
// divisors.
case Instruction::Select: {
VPValue *VPV = VPI->getVPSingleValue();
- if (VPV->getNumUsers() == 1) {
+ if (VPV->getNumUses() == 1) {
if (auto *WR = dyn_cast<VPWidenRecipe>(*VPV->user_begin())) {
switch (WR->getOpcode()) {
case Instruction::UDiv:
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index f972efa07eb7e..1d51bef68dd60 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1053,25 +1053,25 @@ const VPRegionBlock *VPlan::getVectorLoopRegion() const {
void VPlan::printLiveIns(raw_ostream &O) const {
VPSlotTracker SlotTracker(this);
- if (VF.getNumUsers() > 0) {
+ if (VF.getNumUses() > 0) {
O << "\nLive-in ";
VF.printAsOperand(O, SlotTracker);
O << " = VF";
}
- if (VFxUF.getNumUsers() > 0) {
+ if (VFxUF.getNumUses() > 0) {
O << "\nLive-in ";
VFxUF.printAsOperand(O, SlotTracker);
O << " = VF * UF";
}
- if (VectorTripCount.getNumUsers() > 0) {
+ if (VectorTripCount.getNumUses() > 0) {
O << "\nLive-in ";
VectorTripCount.printAsOperand(O, SlotTracker);
O << " = vector-trip-count";
}
- if (BackedgeTakenCount && BackedgeTakenCount->getNumUsers()) {
+ if (BackedgeTakenCount && BackedgeTakenCount->getNumUses()) {
O << "\nLive-in ";
BackedgeTakenCount->printAsOperand(O, SlotTracker);
O << " = backedge-taken count";
@@ -1413,7 +1413,7 @@ void VPValue::replaceUsesWithIf(
if (this == New)
return;
- for (unsigned J = 0; J < getNumUsers();) {
+ for (unsigned J = 0; J < getNumUses();) {
VPUser *User = Users[J];
bool RemovedUser = false;
for (unsigned I = 0, E = User->getNumOperands(); I < E; ++I) {
@@ -1489,9 +1489,9 @@ void VPSlotTracker::assignName(const VPValue *V) {
}
void VPSlotTracker::assignNames(const VPlan &Plan) {
- if (Plan.VF.getNumUsers() > 0)
+ if (Plan.VF.getNumUses() > 0)
assignName(&Plan.VF);
- if (Plan.VFxUF.getNumUsers() > 0)
+ if (Plan.VFxUF.getNumUses() > 0)
assignName(&Plan.VFxUF);
assignName(&Plan.VectorTripCount);
if (Plan.BackedgeTakenCount)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 33bcb49b81740..3d890dfedc2e6 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -4102,7 +4102,7 @@ class VPlan {
/// Resets the trip count for the VPlan. The caller must make sure all uses of
/// the original trip count have been replaced.
void resetTripCount(VPValue *NewTripCount) {
- assert(TripCount && NewTripCount && TripCount->getNumUsers() == 0 &&
+ assert(TripCount && NewTripCount && TripCount->getNumUses() == 0 &&
"TripCount must be set when resetting");
TripCount = NewTripCount;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 4a8b4b8d04840..2e1b9407d42b0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -801,7 +801,7 @@ bool VPlanTransforms::handleMaxMinNumReductions(VPlan &Plan) {
if (VecV == RdxResult)
continue;
if (auto *DerivedIV = dyn_cast<VPDerivedIVRecipe>(VecV)) {
- if (DerivedIV->getNumUsers() == 1 &&
+ if (DerivedIV->getNumUses() == 1 &&
DerivedIV->getOperand(1) == &Plan.getVectorTripCount()) {
auto *NewSel = Builder.createSelect(AnyNaN, Plan.getCanonicalIV(),
&Plan.getVectorTripCount());
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 4890da7e28783..9c0dc8966a4ad 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -2228,7 +2228,7 @@ InstructionCost VPWidenCastRecipe::computeCost(ElementCount VF,
TTI::CastContextHint CCH = TTI::CastContextHint::None;
// For Trunc/FPTrunc, get the context from the only user.
if ((Opcode == Instruction::Trunc || Opcode == Instruction::FPTrunc) &&
- !hasMoreThanOneUniqueUser() && getNumUsers() > 0) {
+ !hasMoreThanOneUniqueUser() && getNumUses() > 0) {
if (auto *StoreRecipe = dyn_cast<VPRecipeBase>(*user_begin()))
CCH = ComputeCCH(StoreRecipe);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 56175e7f18145..e24035dbd6d80 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -311,7 +311,7 @@ static bool mergeReplicateRegionsIntoSuccessors(VPlan &Plan) {
});
// Remove phi recipes that are unused after merging the regions.
- if (Phi1ToMove.getVPSingleValue()->getNumUsers() == 0) {
+ if (Phi1ToMove.getVPSingleValue()->getNumUses() == 0) {
Phi1ToMove.eraseFromParent();
continue;
}
@@ -358,7 +358,7 @@ static VPRegionBlock *createReplicateRegion(VPReplicateRecipe *PredRecipe,
Plan.createVPBasicBlock(Twine(RegionName) + ".if", RecipeWithoutMask);
VPPredInstPHIRecipe *PHIRecipe = nullptr;
- if (PredRecipe->getNumUsers() != 0) {
+ if (PredRecipe->getNumUses() != 0) {
PHIRecipe = new VPPredInstPHIRecipe(RecipeWithoutMask,
RecipeWithoutMask->getDebugLoc());
PredRecipe->replaceAllUsesWith(PHIRecipe);
@@ -542,7 +542,7 @@ static bool isDeadRecipe(VPRecipeBase &R) {
// Recipe is dead if no user keeps the recipe alive.
return all_of(R.definedValues(),
- [](VPValue *V) { return V->getNumUsers() == 0; });
+ [](VPValue *V) { return V->getNumUses() == 0; });
}
void VPlanTransforms::removeDeadRecipes(VPlan &Plan) {
@@ -558,11 +558,11 @@ void VPlanTransforms::removeDeadRecipes(VPlan &Plan) {
// Check if R is a dead VPPhi <-> update cycle and remove it.
auto *PhiR = dyn_cast<VPPhi>(&R);
- if (!PhiR || PhiR->getNumOperands() != 2 || PhiR->getNumUsers() != 1)
+ if (!PhiR || PhiR->getNumOperands() != 2 || PhiR->getNumUses() != 1)
continue;
VPValue *Incoming = PhiR->getOperand(1);
if (*PhiR->user_begin() != Incoming->getDefiningRecipe() ||
- Incoming->getNumUsers() != 1)
+ Incoming->getNumUses() != 1)
continue;
PhiR->replaceAllUsesWith(PhiR->getOperand(0));
PhiR->eraseFromParent();
@@ -653,7 +653,7 @@ static void legalizeAndOptimizeInductions(VPlan &Plan) {
auto *RepR = dyn_cast<VPReplicateRecipe>(U);
// Skip recipes that shouldn't be narrowed.
if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) ||
- Def->getNumUsers() == 0 || !Def->getUnderlyingValue() ||
+ Def->getNumUses() == 0 || !Def->getUnderlyingValue() ||
(RepR && (RepR->isSingleScalar() || RepR->isPredicated())))
continue;
@@ -1321,7 +1321,7 @@ static void simplifyBlends(VPlan &Plan) {
// TODO: Find the most expensive mask that can be deadcoded, or a mask
// that's used by multiple blends where it can be removed from them all.
VPValue *Mask = Blend->getMask(I);
- if (Mask->getNumUsers() == 1 && !match(Mask, m_False())) {
+ if (Mask->getNumUses() == 1 && !match(Mask, m_False())) {
StartIndex = I;
break;
}
@@ -1357,7 +1357,7 @@ static void simplifyBlends(VPlan &Plan) {
NewBlend->setOperand(0, Inc1);
NewBlend->setOperand(1, Inc0);
NewBlend->setOperand(2, NewMask);
- if (OldMask->getNumUsers() == 0)
+ if (OldMask->getNumUses() == 0)
cast<VPInstruction>(OldMask)->eraseFromParent();
}
}
@@ -2341,7 +2341,7 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
ToErase.push_back(CurRecipe);
}
// Remove dead EVL mask.
- if (EVLMask->getNumUsers() == 0)
+ if (EVLMask->getNumUses() == 0)
ToErase.push_back(EVLMask->getDefiningRecipe());
for (VPRecipeBase *R : reverse(ToErase)) {
@@ -3256,7 +3256,7 @@ void VPlanTransforms::materializeBroadcasts(VPlan &Plan) {
#endif
SmallVector<VPValue *> VPValues;
- if (Plan.getOrCreateBackedgeTakenCount()->getNumUsers() > 0)
+ if (Plan.getOrCreateBackedgeTakenCount()->getNumUses() > 0)
VPValues.push_back(Plan.getOrCreateBackedgeTakenCount());
append_range(VPValues, Plan.getLiveIns());
for (VPRecipeBase &R : *Plan.getEntry())
@@ -3325,7 +3325,7 @@ void VPlanTransforms::materializeConstantVectorTripCount(
void VPlanTransforms::materializeBackedgeTakenCount(VPlan &Plan,
VPBasicBlock *VectorPH) {
VPValue *BTC = Plan.getOrCreateBackedgeTakenCount();
- if (BTC->getNumUsers() == 0)
+ if (BTC->getNumUses() == 0)
return;
VPBuilder Builder(VectorPH, VectorPH->begin());
@@ -3391,7 +3391,7 @@ void VPlanTransforms::materializeVectorTripCount(VPlan &Plan,
assert(VectorTC.isLiveIn() && "vector-trip-count must be a live-in");
// There's nothing to do if there are no users of the vector trip count or its
// IR value has already been set.
- if (VectorTC.getNumUsers() == 0 || VectorTC.getLiveInIRValue())
+ if (VectorTC.getNumUses() == 0 || VectorTC.getLiveInIRValue())
return;
VPValue *TC = Plan.getTripCount();
@@ -3456,7 +3456,7 @@ void VPlanTransforms::materializeVFAndVFxUF(VPlan &Plan, VPBasicBlock *VectorPH,
// If there are no users of the runtime VF, compute VFxUF by constant folding
// the multiplication of VF and UF.
- if (VF.getNumUsers() == 0) {
+ if (VF.getNumUses() == 0) {
VPValue *RuntimeVFxUF =
Builder.createElementCount(TCTy, VFEC * Plan.getUF());
VFxUF.replaceAllUsesWith(RuntimeVFxUF);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
index 4bcde8cd5d42a..b88a9d71c9245 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
@@ -535,7 +535,7 @@ void VPlanTransforms::replicateByVF(VPlan &Plan, ElementCount VF) {
continue;
VPBuilder Builder(RepR);
- if (RepR->getNumUsers() == 0) {
+ if (RepR->getNumUses() == 0) {
if (isa<StoreInst>(RepR->getUnderlyingInstr()) &&
vputils::isSingleScalar(RepR->getOperand(1))) {
// Stores to invariant addresses need to store the last lane only.
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index 24f6d61512ef6..eed7c86561788 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -110,13 +110,18 @@ class LLVM_ABI_FOR_TEST VPValue {
void dump() const;
#endif
- unsigned getNumUsers() const { return Users.size(); }
- void addUser(VPUser &User) { Users.push_back(&User); }
+ /// Returns the number of uses of this VPValue. The same user could appear
+ /// more than once in the list due to this value being used for multiple
+ /// operands in a recipe.
+ unsigned getNumUses() const { return Users.size(); }
- /// Remove a single \p User from the list of users.
- void removeUser(VPUser &User) {
+ /// Adds a use of this value to the list.
+ void addUse(VPUser &User) { Users.push_back(&User); }
+
+ /// Remove the first instance of \p User from the list of uses.
+ void removeFirstUse(VPUser &User) {
// The same user can be added multiple times, e.g. because the same VPValue
- // is used twice by the same VPUser. Remove a single one.
+ // is used twice by the same VPUser. Remove the first one.
auto *I = find(Users, &User);
if (I != Users.end())
Users.erase(I);
@@ -138,7 +143,7 @@ class LLVM_ABI_FOR_TEST VPValue {
/// Returns true if the value has more than one unique user.
bool hasMoreThanOneUniqueUser() const {
- if (getNumUsers() == 0)
+ if (getNumUses() == 0)
return false;
// Check if all users match the first user.
@@ -203,7 +208,7 @@ class VPUser {
/// Removes the operand at index \p Idx. This also removes the VPUser from the
/// use-list of the operand.
void removeOperand(unsigned Idx) {
- getOperand(Idx)->removeUser(*this);
+ getOperand(Idx)->removeFirstUse(*this);
Operands.erase(Operands.begin() + Idx);
}
@@ -224,12 +229,12 @@ class VPUser {
VPUser &operator=(const VPUser &) = delete;
virtual ~VPUser() {
for (VPValue *Op : operands())
- Op->removeUser(*this);
+ Op->removeFirstUse(*this);
}
void addOperand(VPValue *Operand) {
Operands.push_back(Operand);
- Operand->addUser(*this);
+ Operand->addUse(*this);
}
unsigned getNumOperands() const { return Operands.size(); }
@@ -239,9 +244,9 @@ class VPUser {
}
void setOperand(unsigned I, VPValue *New) {
- Operands[I]->removeUser(*this);
+ Operands[I]->removeFirstUse(*this);
Operands[I] = New;
- New->addUser(*this);
+ New->addUse(*this);
}
/// Swap operands of the VPUser. It must have exactly 2 operands.
@@ -382,7 +387,7 @@ class VPDef {
for (VPValue *D : make_early_inc_range(DefinedValues)) {
assert(D->Def == this &&
"all defined VPValues should point to the containing VPDef");
- assert(D->getNumUsers() == 0 &&
+ assert(D->getNumUses() == 0 &&
"all defined VPValues should have no more users");
D->Def = nullptr;
delete D;
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index e25ffe135418e..d5ff2d14f5d0d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -197,8 +197,8 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
}
// EVLIVIncrement is only used by EVLIV & BranchOnCount.
// Having more than two users is unexpected.
- if ((I->getNumUsers() != 1) &&
- (I->getNumUsers() != 2 || none_of(I->users(), [&I](VPUser *U) {
+ if ((I->getNumUses() != 1) &&
+ (I->getNumUses() != 2 || none_of(I->users(), [&I](VPUser *U) {
using namespace llvm::VPlanPatternMatch;
return match(U, m_BranchOnCount(m_Specific(I), m_VPValue()));
}))) {
|
|
@llvm/pr-subscribers-vectorizers Author: David Sherwood (david-arm) ChangesThe Users list in VPValue is really just a Use list because the same User can appear several times in the list, reflecting the fact the VPValue can be used for more than one operand of a recipe. In such a scenario there is really just one user with multiple uses of a given VPValue. See the documentation for the IR routine Value::hasOneUser(): I've renamed this function, along with addUser and removeUser to mirror the normal IR terminology. Full diff: https://github.com/llvm/llvm-project/pull/155397.diff 9 Files Affected:
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index fff4ad752e180..7aa6706fc0bb7 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -4290,7 +4290,7 @@ VectorizationFactor LoopVectorizationPlanner::selectVectorizationFactor() {
// divisors.
case Instruction::Select: {
VPValue *VPV = VPI->getVPSingleValue();
- if (VPV->getNumUsers() == 1) {
+ if (VPV->getNumUses() == 1) {
if (auto *WR = dyn_cast<VPWidenRecipe>(*VPV->user_begin())) {
switch (WR->getOpcode()) {
case Instruction::UDiv:
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index f972efa07eb7e..1d51bef68dd60 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1053,25 +1053,25 @@ const VPRegionBlock *VPlan::getVectorLoopRegion() const {
void VPlan::printLiveIns(raw_ostream &O) const {
VPSlotTracker SlotTracker(this);
- if (VF.getNumUsers() > 0) {
+ if (VF.getNumUses() > 0) {
O << "\nLive-in ";
VF.printAsOperand(O, SlotTracker);
O << " = VF";
}
- if (VFxUF.getNumUsers() > 0) {
+ if (VFxUF.getNumUses() > 0) {
O << "\nLive-in ";
VFxUF.printAsOperand(O, SlotTracker);
O << " = VF * UF";
}
- if (VectorTripCount.getNumUsers() > 0) {
+ if (VectorTripCount.getNumUses() > 0) {
O << "\nLive-in ";
VectorTripCount.printAsOperand(O, SlotTracker);
O << " = vector-trip-count";
}
- if (BackedgeTakenCount && BackedgeTakenCount->getNumUsers()) {
+ if (BackedgeTakenCount && BackedgeTakenCount->getNumUses()) {
O << "\nLive-in ";
BackedgeTakenCount->printAsOperand(O, SlotTracker);
O << " = backedge-taken count";
@@ -1413,7 +1413,7 @@ void VPValue::replaceUsesWithIf(
if (this == New)
return;
- for (unsigned J = 0; J < getNumUsers();) {
+ for (unsigned J = 0; J < getNumUses();) {
VPUser *User = Users[J];
bool RemovedUser = false;
for (unsigned I = 0, E = User->getNumOperands(); I < E; ++I) {
@@ -1489,9 +1489,9 @@ void VPSlotTracker::assignName(const VPValue *V) {
}
void VPSlotTracker::assignNames(const VPlan &Plan) {
- if (Plan.VF.getNumUsers() > 0)
+ if (Plan.VF.getNumUses() > 0)
assignName(&Plan.VF);
- if (Plan.VFxUF.getNumUsers() > 0)
+ if (Plan.VFxUF.getNumUses() > 0)
assignName(&Plan.VFxUF);
assignName(&Plan.VectorTripCount);
if (Plan.BackedgeTakenCount)
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 33bcb49b81740..3d890dfedc2e6 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -4102,7 +4102,7 @@ class VPlan {
/// Resets the trip count for the VPlan. The caller must make sure all uses of
/// the original trip count have been replaced.
void resetTripCount(VPValue *NewTripCount) {
- assert(TripCount && NewTripCount && TripCount->getNumUsers() == 0 &&
+ assert(TripCount && NewTripCount && TripCount->getNumUses() == 0 &&
"TripCount must be set when resetting");
TripCount = NewTripCount;
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
index 4a8b4b8d04840..2e1b9407d42b0 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanConstruction.cpp
@@ -801,7 +801,7 @@ bool VPlanTransforms::handleMaxMinNumReductions(VPlan &Plan) {
if (VecV == RdxResult)
continue;
if (auto *DerivedIV = dyn_cast<VPDerivedIVRecipe>(VecV)) {
- if (DerivedIV->getNumUsers() == 1 &&
+ if (DerivedIV->getNumUses() == 1 &&
DerivedIV->getOperand(1) == &Plan.getVectorTripCount()) {
auto *NewSel = Builder.createSelect(AnyNaN, Plan.getCanonicalIV(),
&Plan.getVectorTripCount());
diff --git a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
index 4890da7e28783..9c0dc8966a4ad 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -2228,7 +2228,7 @@ InstructionCost VPWidenCastRecipe::computeCost(ElementCount VF,
TTI::CastContextHint CCH = TTI::CastContextHint::None;
// For Trunc/FPTrunc, get the context from the only user.
if ((Opcode == Instruction::Trunc || Opcode == Instruction::FPTrunc) &&
- !hasMoreThanOneUniqueUser() && getNumUsers() > 0) {
+ !hasMoreThanOneUniqueUser() && getNumUses() > 0) {
if (auto *StoreRecipe = dyn_cast<VPRecipeBase>(*user_begin()))
CCH = ComputeCCH(StoreRecipe);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index 56175e7f18145..e24035dbd6d80 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -311,7 +311,7 @@ static bool mergeReplicateRegionsIntoSuccessors(VPlan &Plan) {
});
// Remove phi recipes that are unused after merging the regions.
- if (Phi1ToMove.getVPSingleValue()->getNumUsers() == 0) {
+ if (Phi1ToMove.getVPSingleValue()->getNumUses() == 0) {
Phi1ToMove.eraseFromParent();
continue;
}
@@ -358,7 +358,7 @@ static VPRegionBlock *createReplicateRegion(VPReplicateRecipe *PredRecipe,
Plan.createVPBasicBlock(Twine(RegionName) + ".if", RecipeWithoutMask);
VPPredInstPHIRecipe *PHIRecipe = nullptr;
- if (PredRecipe->getNumUsers() != 0) {
+ if (PredRecipe->getNumUses() != 0) {
PHIRecipe = new VPPredInstPHIRecipe(RecipeWithoutMask,
RecipeWithoutMask->getDebugLoc());
PredRecipe->replaceAllUsesWith(PHIRecipe);
@@ -542,7 +542,7 @@ static bool isDeadRecipe(VPRecipeBase &R) {
// Recipe is dead if no user keeps the recipe alive.
return all_of(R.definedValues(),
- [](VPValue *V) { return V->getNumUsers() == 0; });
+ [](VPValue *V) { return V->getNumUses() == 0; });
}
void VPlanTransforms::removeDeadRecipes(VPlan &Plan) {
@@ -558,11 +558,11 @@ void VPlanTransforms::removeDeadRecipes(VPlan &Plan) {
// Check if R is a dead VPPhi <-> update cycle and remove it.
auto *PhiR = dyn_cast<VPPhi>(&R);
- if (!PhiR || PhiR->getNumOperands() != 2 || PhiR->getNumUsers() != 1)
+ if (!PhiR || PhiR->getNumOperands() != 2 || PhiR->getNumUses() != 1)
continue;
VPValue *Incoming = PhiR->getOperand(1);
if (*PhiR->user_begin() != Incoming->getDefiningRecipe() ||
- Incoming->getNumUsers() != 1)
+ Incoming->getNumUses() != 1)
continue;
PhiR->replaceAllUsesWith(PhiR->getOperand(0));
PhiR->eraseFromParent();
@@ -653,7 +653,7 @@ static void legalizeAndOptimizeInductions(VPlan &Plan) {
auto *RepR = dyn_cast<VPReplicateRecipe>(U);
// Skip recipes that shouldn't be narrowed.
if (!Def || !isa<VPReplicateRecipe, VPWidenRecipe>(Def) ||
- Def->getNumUsers() == 0 || !Def->getUnderlyingValue() ||
+ Def->getNumUses() == 0 || !Def->getUnderlyingValue() ||
(RepR && (RepR->isSingleScalar() || RepR->isPredicated())))
continue;
@@ -1321,7 +1321,7 @@ static void simplifyBlends(VPlan &Plan) {
// TODO: Find the most expensive mask that can be deadcoded, or a mask
// that's used by multiple blends where it can be removed from them all.
VPValue *Mask = Blend->getMask(I);
- if (Mask->getNumUsers() == 1 && !match(Mask, m_False())) {
+ if (Mask->getNumUses() == 1 && !match(Mask, m_False())) {
StartIndex = I;
break;
}
@@ -1357,7 +1357,7 @@ static void simplifyBlends(VPlan &Plan) {
NewBlend->setOperand(0, Inc1);
NewBlend->setOperand(1, Inc0);
NewBlend->setOperand(2, NewMask);
- if (OldMask->getNumUsers() == 0)
+ if (OldMask->getNumUses() == 0)
cast<VPInstruction>(OldMask)->eraseFromParent();
}
}
@@ -2341,7 +2341,7 @@ static void transformRecipestoEVLRecipes(VPlan &Plan, VPValue &EVL) {
ToErase.push_back(CurRecipe);
}
// Remove dead EVL mask.
- if (EVLMask->getNumUsers() == 0)
+ if (EVLMask->getNumUses() == 0)
ToErase.push_back(EVLMask->getDefiningRecipe());
for (VPRecipeBase *R : reverse(ToErase)) {
@@ -3256,7 +3256,7 @@ void VPlanTransforms::materializeBroadcasts(VPlan &Plan) {
#endif
SmallVector<VPValue *> VPValues;
- if (Plan.getOrCreateBackedgeTakenCount()->getNumUsers() > 0)
+ if (Plan.getOrCreateBackedgeTakenCount()->getNumUses() > 0)
VPValues.push_back(Plan.getOrCreateBackedgeTakenCount());
append_range(VPValues, Plan.getLiveIns());
for (VPRecipeBase &R : *Plan.getEntry())
@@ -3325,7 +3325,7 @@ void VPlanTransforms::materializeConstantVectorTripCount(
void VPlanTransforms::materializeBackedgeTakenCount(VPlan &Plan,
VPBasicBlock *VectorPH) {
VPValue *BTC = Plan.getOrCreateBackedgeTakenCount();
- if (BTC->getNumUsers() == 0)
+ if (BTC->getNumUses() == 0)
return;
VPBuilder Builder(VectorPH, VectorPH->begin());
@@ -3391,7 +3391,7 @@ void VPlanTransforms::materializeVectorTripCount(VPlan &Plan,
assert(VectorTC.isLiveIn() && "vector-trip-count must be a live-in");
// There's nothing to do if there are no users of the vector trip count or its
// IR value has already been set.
- if (VectorTC.getNumUsers() == 0 || VectorTC.getLiveInIRValue())
+ if (VectorTC.getNumUses() == 0 || VectorTC.getLiveInIRValue())
return;
VPValue *TC = Plan.getTripCount();
@@ -3456,7 +3456,7 @@ void VPlanTransforms::materializeVFAndVFxUF(VPlan &Plan, VPBasicBlock *VectorPH,
// If there are no users of the runtime VF, compute VFxUF by constant folding
// the multiplication of VF and UF.
- if (VF.getNumUsers() == 0) {
+ if (VF.getNumUses() == 0) {
VPValue *RuntimeVFxUF =
Builder.createElementCount(TCTy, VFEC * Plan.getUF());
VFxUF.replaceAllUsesWith(RuntimeVFxUF);
diff --git a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
index 4bcde8cd5d42a..b88a9d71c9245 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanUnroll.cpp
@@ -535,7 +535,7 @@ void VPlanTransforms::replicateByVF(VPlan &Plan, ElementCount VF) {
continue;
VPBuilder Builder(RepR);
- if (RepR->getNumUsers() == 0) {
+ if (RepR->getNumUses() == 0) {
if (isa<StoreInst>(RepR->getUnderlyingInstr()) &&
vputils::isSingleScalar(RepR->getOperand(1))) {
// Stores to invariant addresses need to store the last lane only.
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index 24f6d61512ef6..eed7c86561788 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -110,13 +110,18 @@ class LLVM_ABI_FOR_TEST VPValue {
void dump() const;
#endif
- unsigned getNumUsers() const { return Users.size(); }
- void addUser(VPUser &User) { Users.push_back(&User); }
+ /// Returns the number of uses of this VPValue. The same user could appear
+ /// more than once in the list due to this value being used for multiple
+ /// operands in a recipe.
+ unsigned getNumUses() const { return Users.size(); }
- /// Remove a single \p User from the list of users.
- void removeUser(VPUser &User) {
+ /// Adds a use of this value to the list.
+ void addUse(VPUser &User) { Users.push_back(&User); }
+
+ /// Remove the first instance of \p User from the list of uses.
+ void removeFirstUse(VPUser &User) {
// The same user can be added multiple times, e.g. because the same VPValue
- // is used twice by the same VPUser. Remove a single one.
+ // is used twice by the same VPUser. Remove the first one.
auto *I = find(Users, &User);
if (I != Users.end())
Users.erase(I);
@@ -138,7 +143,7 @@ class LLVM_ABI_FOR_TEST VPValue {
/// Returns true if the value has more than one unique user.
bool hasMoreThanOneUniqueUser() const {
- if (getNumUsers() == 0)
+ if (getNumUses() == 0)
return false;
// Check if all users match the first user.
@@ -203,7 +208,7 @@ class VPUser {
/// Removes the operand at index \p Idx. This also removes the VPUser from the
/// use-list of the operand.
void removeOperand(unsigned Idx) {
- getOperand(Idx)->removeUser(*this);
+ getOperand(Idx)->removeFirstUse(*this);
Operands.erase(Operands.begin() + Idx);
}
@@ -224,12 +229,12 @@ class VPUser {
VPUser &operator=(const VPUser &) = delete;
virtual ~VPUser() {
for (VPValue *Op : operands())
- Op->removeUser(*this);
+ Op->removeFirstUse(*this);
}
void addOperand(VPValue *Operand) {
Operands.push_back(Operand);
- Operand->addUser(*this);
+ Operand->addUse(*this);
}
unsigned getNumOperands() const { return Operands.size(); }
@@ -239,9 +244,9 @@ class VPUser {
}
void setOperand(unsigned I, VPValue *New) {
- Operands[I]->removeUser(*this);
+ Operands[I]->removeFirstUse(*this);
Operands[I] = New;
- New->addUser(*this);
+ New->addUse(*this);
}
/// Swap operands of the VPUser. It must have exactly 2 operands.
@@ -382,7 +387,7 @@ class VPDef {
for (VPValue *D : make_early_inc_range(DefinedValues)) {
assert(D->Def == this &&
"all defined VPValues should point to the containing VPDef");
- assert(D->getNumUsers() == 0 &&
+ assert(D->getNumUses() == 0 &&
"all defined VPValues should have no more users");
D->Def = nullptr;
delete D;
diff --git a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
index e25ffe135418e..d5ff2d14f5d0d 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanVerifier.cpp
@@ -197,8 +197,8 @@ bool VPlanVerifier::verifyEVLRecipe(const VPInstruction &EVL) const {
}
// EVLIVIncrement is only used by EVLIV & BranchOnCount.
// Having more than two users is unexpected.
- if ((I->getNumUsers() != 1) &&
- (I->getNumUsers() != 2 || none_of(I->users(), [&I](VPUser *U) {
+ if ((I->getNumUses() != 1) &&
+ (I->getNumUses() != 2 || none_of(I->users(), [&I](VPUser *U) {
using namespace llvm::VPlanPatternMatch;
return match(U, m_BranchOnCount(m_Specific(I), m_VPValue()));
}))) {
|
sdesmalen-arm
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.
These names seem to better reflect what the code is actually doing.
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.
Hm, this is clearer for some uses, but is inconsistent in other ways, as this doesn't really add a use, it adds a user object of which there are may be multiple and there's only accessors for iterating over users not uses at the moment
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.
IR/Value.h only has a single UseList and it creates iterators for both uses and users from the same UseList. Also, Value.h only has an addUse function - addUser does not exist. I know in other parts of the vectoriser we've tried hard to keep consistent naming schemes and terminology with the IR equivalents, which is what I'm trying to do here. Is the problem you're worried about here that addUse takes a VPUser?
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.
Currently uses/users are handled in VPlan is quite different to Values in LLVM IR, which is different for most other parts of VPlan unfortunately. With the current implementation, we cannot really consistently match LLVM IR Users' interface I think
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.
But what about getNumUsers? Are you saying that in VPlan a User is actually more like a Use? There is ambiguity when getNumUsers returns a value of 2 or more, since the caller has to figure out if this the same user using the value multiple times (for different operands) or different unique users. Would it be useful at least to have both getNumUses and getNumUsers where the latter actually does the work to figure out how many unique users?
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 do think the name removeUser is very misleading because it's not actually removing the user from the list - only the first instance. The caller might be surprised to know the VPUser is still there!
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.
Regardless of consistency with LLVM's Users' interface, the implementation of getNumUsers() and removeUser do something different than what the name suggests and warrant a change. I guess that the name addUser can be argued either way, but overall I'd say that the names suggested in this PR are an improvement.
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've reverted addUser back to addUse.
The Users list in VPValue is really just a Use list because the same User can appear several times in the list, reflecting the fact the VPValue can be used for more than one operand of a recipe. In such a scenario there is really just one *user* with multiple *uses* of a given Value. See the documentation for the IR routine Value::hasOneUser(): --- /// Return true if there is exactly one user of this value. /// /// Note that this is not the same as "has one use". If a value has one use, /// then there certainly is a single user. But if value has several uses, /// it is possible that all uses are in a single user, or not. /// /// This check is potentially costly, since it requires traversing, /// in the worst case, the whole use list of a value. LLVM_ABI bool hasOneUser() const; --- I've renamed this function, along with removeUser to more accurately reflect what the code is doing.
95828ed to
09cebd2
Compare
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.
Thanks for the update, will try to take another look tomorrow or the day after
|
Hi @fhahn, unless you have any objections I plan to land this patch tomorrow morning. |
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.
Ah yes, sorry for the delay! Looking again at the places that need updating, in many of those are interested a few cases
- no users
- one (unique) user
- or more than one (unique) user
Updating them to check the number of uses matches the current return value of getNumUsers, but checking the uses may subtly change what the checks actually guard against (and may unnecessarily pessimize them if we would update getNumUsers to skip duplicates).
I think we should be able to answer the 3 questions above accurately without too much overhead: checking if there is a single unique user requires checking all users are the same, but that should be cheap in practice, as the number of operands of a single user is quite low, which limits the worst case. Making getNumUsers return the non-unique use count in general would be potentially much more expensive, as there could be many different users using a single value.
An alternative may be to add dedicated helpers to answer the 3 different queries, and remove getNumUsers to avoid surprises. I sketched that here: #157956. Together with that, we could still provide getNumUses() and use that instead of the helper to check if there are any users.
The Users list in VPValue is really just a Use list because the same User can appear several times in the list, reflecting the fact the VPValue can be used for more than one operand of a recipe. In such a scenario there is really just one user with multiple uses of a given VPValue. See the documentation for the IR routine Value::hasOneUser():
I've renamed this function, along with addUser and removeUser to mirror the normal IR terminology.