[llvm] abd36fe - [VPlan] Introduce code to limit querying VPValues using IR references.
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 30 00:12:51 PDT 2021
Author: Florian Hahn
Date: 2021-08-30T09:12:09+02:00
New Revision: abd36fe512a6ba8de51bca7911322ae8fa1b0a78
URL: https://github.com/llvm/llvm-project/commit/abd36fe512a6ba8de51bca7911322ae8fa1b0a78
DIFF: https://github.com/llvm/llvm-project/commit/abd36fe512a6ba8de51bca7911322ae8fa1b0a78.diff
LOG: [VPlan] Introduce code to limit querying VPValues using IR references.
After applying VPlan-to-VPlan transformations, using IR references to
query VPlan values may be incorrect, as the IR is not in sync with the
VPlan any longer.
To better detect such mis-matches, this patch introduces a new flag to
VPlans to indicate whether it is safe to query VPValues using IR values.
getVPValue is updated to assert if it is called when the flag indicates
it is not safe any longer.
There is an escape hatch via an extra argument, because there are 3
places that need to be fixed first. Those are
1. truncateToMinimalBitwidths
2. clearReductionWrapFlags
3. fixLCSSAPHIs
As a first step, this flag will help preventing new code from violating
this property.
Any suggestions with respect to naming very welcome!
Reviewed By: Ayal
Differential Revision: https://reviews.llvm.org/D108573
Added:
Modified:
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/VPlan.h
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index d13a75ab94736..260e1824870c4 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -3989,7 +3989,8 @@ void InnerLoopVectorizer::truncateToMinimalBitwidths(VPTransformState &State) {
// If the value wasn't vectorized, we must maintain the original scalar
// type. The absence of the value from State indicates that it
// wasn't vectorized.
- VPValue *Def = State.Plan->getVPValue(KV.first);
+ // FIXME: Should not rely on getVPValue at this point.
+ VPValue *Def = State.Plan->getVPValue(KV.first, true);
if (!State.hasAnyVectorValue(Def))
continue;
for (unsigned Part = 0; Part < UF; ++Part) {
@@ -4096,7 +4097,8 @@ void InnerLoopVectorizer::truncateToMinimalBitwidths(VPTransformState &State) {
// If the value wasn't vectorized, we must maintain the original scalar
// type. The absence of the value from State indicates that it
// wasn't vectorized.
- VPValue *Def = State.Plan->getVPValue(KV.first);
+ // FIXME: Should not rely on getVPValue at this point.
+ VPValue *Def = State.Plan->getVPValue(KV.first, true);
if (!State.hasAnyVectorValue(Def))
continue;
for (unsigned Part = 0; Part < UF; ++Part) {
@@ -4482,7 +4484,8 @@ void InnerLoopVectorizer::clearReductionWrapFlags(const RecurrenceDescriptor &Rd
Instruction *Cur = Worklist.pop_back_val();
if (isa<OverflowingBinaryOperator>(Cur))
for (unsigned Part = 0; Part < UF; ++Part) {
- Value *V = State.get(State.Plan->getVPValue(Cur), Part);
+ // FIXME: Should not rely on getVPValue at this point.
+ Value *V = State.get(State.Plan->getVPValue(Cur, true), Part);
cast<Instruction>(V)->dropPoisonGeneratingFlags();
}
@@ -4513,11 +4516,12 @@ void InnerLoopVectorizer::fixLCSSAPHIs(VPTransformState &State) {
// Can be a loop invariant incoming value or the last scalar value to be
// extracted from the vectorized loop.
+ // FIXME: Should not rely on getVPValue at this point.
Builder.SetInsertPoint(LoopMiddleBlock->getTerminator());
Value *lastIncomingValue =
OrigLoop->isLoopInvariant(IncomingValue)
? IncomingValue
- : State.get(State.Plan->getVPValue(IncomingValue),
+ : State.get(State.Plan->getVPValue(IncomingValue, true),
VPIteration(UF - 1, Lane));
LCSSAPhi.addIncoming(lastIncomingValue, LoopMiddleBlock);
}
@@ -9480,6 +9484,10 @@ VPlanPtr LoopVectorizationPlanner::buildVPlanWithVPRecipes(
}
}
+ // From this point onwards, VPlan-to-VPlan transformations may change the plan
+ // in ways that accessing values using original IR values is incorrect.
+ Plan->disableValue2VPValue();
+
VPlanTransforms::sinkScalarOperands(*Plan);
VPlanTransforms::mergeReplicateRegions(*Plan);
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 34fa2bfc5079a..021bd97f0a1f3 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -2100,6 +2100,10 @@ class VPlan {
/// Holds the VPLoopInfo analysis for this VPlan.
VPLoopInfo VPLInfo;
+ /// Indicates whether it is safe use the Value2VPValue mapping or if the
+ /// mapping cannot be used any longer, because it is stale.
+ bool Value2VPValueEnabled = true;
+
public:
VPlan(VPBlockBase *Entry = nullptr) : Entry(Entry) {
if (Entry)
@@ -2141,6 +2145,10 @@ class VPlan {
return BackedgeTakenCount;
}
+ /// Mark the plan to indicate that using Value2VPValue is not safe any
+ /// longer, because it may be stale.
+ void disableValue2VPValue() { Value2VPValueEnabled = false; }
+
void addVF(ElementCount VF) { VFs.insert(VF); }
bool hasVF(ElementCount VF) { return VFs.count(VF); }
@@ -2154,6 +2162,8 @@ class VPlan {
void addExternalDef(VPValue *VPVal) { VPExternalDefs.insert(VPVal); }
void addVPValue(Value *V) {
+ assert(Value2VPValueEnabled &&
+ "IR value to VPValue mapping may be out of date!");
assert(V && "Trying to add a null Value to VPlan");
assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
VPValue *VPV = new VPValue(V);
@@ -2162,25 +2172,39 @@ class VPlan {
}
void addVPValue(Value *V, VPValue *VPV) {
+ assert(Value2VPValueEnabled && "Value2VPValue mapping may be out of date!");
assert(V && "Trying to add a null Value to VPlan");
assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
Value2VPValue[V] = VPV;
}
- VPValue *getVPValue(Value *V) {
+ /// Returns the VPValue for \p V. \p OverrideAllowed can be used to disable
+ /// checking whether it is safe to query VPValues using IR Values.
+ VPValue *getVPValue(Value *V, bool OverrideAllowed = false) {
+ assert((OverrideAllowed || isa<Constant>(V) || Value2VPValueEnabled) &&
+ "Value2VPValue mapping may be out of date!");
assert(V && "Trying to get the VPValue of a null Value");
assert(Value2VPValue.count(V) && "Value does not exist in VPlan");
return Value2VPValue[V];
}
- VPValue *getOrAddVPValue(Value *V) {
+ /// Gets the VPValue or adds a new one (if none exists yet) for \p V. \p
+ /// OverrideAllowed can be used to disable checking whether it is safe to
+ /// query VPValues using IR Values.
+ VPValue *getOrAddVPValue(Value *V, bool OverrideAllowed = false) {
+ assert((OverrideAllowed || isa<Constant>(V) || Value2VPValueEnabled) &&
+ "Value2VPValue mapping may be out of date!");
assert(V && "Trying to get or add the VPValue of a null Value");
if (!Value2VPValue.count(V))
addVPValue(V);
return getVPValue(V);
}
- void removeVPValueFor(Value *V) { Value2VPValue.erase(V); }
+ void removeVPValueFor(Value *V) {
+ assert(Value2VPValueEnabled &&
+ "IR value to VPValue mapping may be out of date!");
+ Value2VPValue.erase(V);
+ }
/// Return the VPLoopInfo analysis for this VPlan.
VPLoopInfo &getVPLoopInfo() { return VPLInfo; }
More information about the llvm-commits
mailing list