[BasicAA] Refactor offset-based heuristics in aliasGEP, unify style (NFC) - #218687
[BasicAA] Refactor offset-based heuristics in aliasGEP, unify style (NFC)#218687antoniofrighetto wants to merge 2 commits into
aliasGEP, unify style (NFC)#218687Conversation
|
@llvm/pr-subscribers-llvm-analysis Author: Antonio Frighetto (antoniofrighetto) ChangesPart of the offset-based reasoning in Minor opportunity to modernize code style where possible. Full diff: https://github.com/llvm/llvm-project/pull/218687.diff 2 Files Affected:
diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
index e42792548e5eb..da81bdec88f45 100644
--- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h
@@ -109,6 +109,12 @@ class BasicAAResult : public AAResultBase {
private:
struct DecomposedGEP;
+ /// Results of analyzing variable GEP indices for offset-based disambiguation.
+ struct VariableGEPOffsetInfo {
+ APInt GCD;
+ ConstantRange OffsetRange;
+ };
+
/// Tracks instructions visited by pointsToConstantMemory.
SmallPtrSet<const Value *, 16> Visited;
@@ -116,6 +122,19 @@ class BasicAAResult : public AAResultBase {
DecomposeGEPExpression(const Value *V, const DataLayout &DL,
AssumptionCache *AC, DominatorTree *DT);
+ /// Analyze the variable indices of a decomposed GEP, computing the GCD
+ /// that each Scale*V term is a multiple of, and an approximate range of
+ /// possible total offsets.
+ VariableGEPOffsetInfo analyzeVariableOffsets(const DecomposedGEP &GEP,
+ DominatorTree *DT);
+
+ /// Try to determine the range of values for VarIndex such that
+ /// VarIndex <= -MinAbsVarIndex || MinAbsVarIndex <= VarIndex, thus
+ /// establishing a minimum absolute value of the variable offset.
+ std::optional<APInt> computeMinAbsVarIndexHeuristic(const DecomposedGEP &GEP,
+ DominatorTree *DT,
+ const AAQueryInfo &AAQI);
+
/// A Heuristic for aliasGEP that searches for a constant offset
/// between the variables.
///
@@ -124,9 +143,10 @@ class BasicAAResult : public AAResultBase {
/// will therefore conservatively refuse to decompose these expressions.
/// However, we know that, for all %x, zext(%x) != zext(%x + 1), even if
/// the addition overflows.
- bool constantOffsetHeuristic(const DecomposedGEP &GEP, LocationSize V1Size,
- LocationSize V2Size, AssumptionCache *AC,
- DominatorTree *DT, const AAQueryInfo &AAQI);
+ bool computeConstantOffsetHeuristic(const DecomposedGEP &GEP,
+ LocationSize V1Size, LocationSize V2Size,
+ AssumptionCache *AC, DominatorTree *DT,
+ const AAQueryInfo &AAQI);
bool isValueEqualInPotentialCycles(const Value *V1, const Value *V2,
const AAQueryInfo &AAQI);
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index e33fde77cf365..95255b4045ee3 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1152,8 +1152,7 @@ AliasResult BasicAAResult::aliasGEP(
// If an inbounds GEP would have to start from an out of bounds address
// for the two to alias, then we can assume noalias.
// TODO: Remove !isScalable() once BasicAA fully support scalable location
- // size
-
+ // size.
if (DecompGEP1.NWFlags.isInBounds() && DecompGEP1.VarIndices.empty() &&
V2Size.hasValue() && !V2Size.isScalable() &&
DecompGEP1.Offset.sge(V2Size.getValue()) &&
@@ -1229,15 +1228,15 @@ AliasResult BasicAAResult::aliasGEP(
return AR;
}
return AliasResult::NoAlias;
- } else {
- // We can use the getVScaleRange to prove that Off >= (CR.upper * LSize).
- ConstantRange CR = getVScaleRange(&F, Off.getBitWidth());
- bool Overflow;
- APInt UpperRange = CR.getUnsignedMax().umul_ov(
- APInt(Off.getBitWidth(), LSize.getKnownMinValue()), Overflow);
- if (!Overflow && Off.uge(UpperRange))
- return AliasResult::NoAlias;
}
+
+ // We can use the getVScaleRange to prove that Off >= (CR.upper * LSize).
+ ConstantRange CR = getVScaleRange(&F, Off.getBitWidth());
+ bool Overflow;
+ APInt UpperRange = CR.getUnsignedMax().umul_ov(
+ APInt(Off.getBitWidth(), LSize.getKnownMinValue()), Overflow);
+ if (!Overflow && Off.uge(UpperRange))
+ return AliasResult::NoAlias;
}
// VScale Alias Analysis - Given one scalable offset between accesses and a
@@ -1285,7 +1284,7 @@ AliasResult BasicAAResult::aliasGEP(
!V2Size.isScalable() && DecompGEP1.Offset.uge(V2Size.getValue()))
return AliasResult::NoAlias;
- // Bail on analysing scalable LocationSize
+ // Bail on analyzing scalable LocationSize.
if (V1Size.isScalable() || V2Size.isScalable())
return AliasResult::MayAlias;
@@ -1296,55 +1295,10 @@ AliasResult BasicAAResult::aliasGEP(
!isUIntN(BW, V1Size.getValue()) || !isUIntN(BW, V2Size.getValue()))
return AliasResult::MayAlias;
- APInt GCD;
- ConstantRange OffsetRange = ConstantRange(DecompGEP1.Offset);
- for (unsigned i = 0, e = DecompGEP1.VarIndices.size(); i != e; ++i) {
- const VariableGEPIndex &Index = DecompGEP1.VarIndices[i];
- const APInt &Scale = Index.Scale;
-
- SimplifyQuery SQ(DL, DT, &AC, Index.CxtI, /*UseInstrInfo=*/true);
- KnownBits Known = computeKnownBits(Index.Val.V, SQ);
-
- APInt ScaleForGCD = Scale;
- if (!Index.IsNSW)
- ScaleForGCD =
- APInt::getOneBitSet(Scale.getBitWidth(), Scale.countr_zero());
-
- // If V has known trailing zeros, V is a multiple of 2^VarTZ, so
- // V*Scale is a multiple of ScaleForGCD * 2^VarTZ. Shift ScaleForGCD
- // left to account for this (trailing zeros compose additively through
- // multiplication, even in Z/2^n).
- unsigned VarTZ = Known.countMinTrailingZeros();
- if (VarTZ > 0) {
- unsigned MaxShift =
- Scale.getBitWidth() - ScaleForGCD.getSignificantBits();
- ScaleForGCD <<= std::min(VarTZ, MaxShift);
- }
-
- if (i == 0)
- GCD = ScaleForGCD.abs();
- else
- GCD = APIntOps::GreatestCommonDivisor(GCD, ScaleForGCD.abs());
-
- ConstantRange CR =
- computeConstantRange(Index.Val.V, /*ForSigned=*/false, SQ);
- CR = CR.intersectWith(
- ConstantRange::fromKnownBits(Known, /* Signed */ true),
- ConstantRange::Signed);
- CR = Index.Val.evaluateWith(CR).sextOrTrunc(OffsetRange.getBitWidth());
-
- assert(OffsetRange.getBitWidth() == Scale.getBitWidth() &&
- "Bit widths are normalized to MaxIndexSize");
- if (Index.IsNSW)
- CR = CR.smul_sat(ConstantRange(Scale));
- else
- CR = CR.smul_fast(ConstantRange(Scale));
-
- if (Index.IsNegated)
- OffsetRange = OffsetRange.sub(CR);
- else
- OffsetRange = OffsetRange.add(CR);
- }
+ // Analyze the variable indices, and compute the GCD that the total
+ // variable offset is guaranteed to be a multiple of, and its approximate
+ // range.
+ auto [GCD, OffsetRange] = analyzeVariableOffsets(DecompGEP1, DT);
// We now have accesses at two offsets from the same base:
// 1. (...)*GCD + DecompGEP1.Offset with size V1Size
@@ -1359,8 +1313,8 @@ AliasResult BasicAAResult::aliasGEP(
(GCD - ModOffset).uge(V1Size.getValue()))
return AliasResult::NoAlias;
- // Compute ranges of potentially accessed bytes for both accesses. If the
- // interseciton is empty, there can be no overlap.
+ // If the ranges of potentially accessed bytes are disjoint, there cannot be
+ // any overlap.
ConstantRange Range1 = OffsetRange.add(
ConstantRange(APInt(BW, 0), APInt(BW, V1Size.getValue())));
ConstantRange Range2 =
@@ -1368,56 +1322,10 @@ AliasResult BasicAAResult::aliasGEP(
if (Range1.intersectWith(Range2).isEmptySet())
return AliasResult::NoAlias;
- // Check if abs(V*Scale) >= abs(Scale) holds in the presence of
- // potentially wrapping math.
- auto MultiplyByScaleNoWrap = [](const VariableGEPIndex &Var) {
- if (Var.IsNSW)
- return true;
-
- int ValOrigBW = Var.Val.V->getType()->getPrimitiveSizeInBits();
- // If Scale is small enough so that abs(V*Scale) >= abs(Scale) holds.
- // The max value of abs(V) is 2^ValOrigBW - 1. Multiplying with a
- // constant smaller than 2^(bitwidth(Val) - ValOrigBW) won't wrap.
- int MaxScaleValueBW = Var.Val.getBitWidth() - ValOrigBW;
- if (MaxScaleValueBW <= 0)
- return false;
- return Var.Scale.ule(
- APInt::getMaxValue(MaxScaleValueBW).zext(Var.Scale.getBitWidth()));
- };
-
- // Try to determine the range of values for VarIndex such that
- // VarIndex <= -MinAbsVarIndex || MinAbsVarIndex <= VarIndex.
- std::optional<APInt> MinAbsVarIndex;
- if (DecompGEP1.VarIndices.size() == 1) {
- // VarIndex = Scale*V.
- const VariableGEPIndex &Var = DecompGEP1.VarIndices[0];
- if (Var.Val.TruncBits == 0 &&
- isKnownNonZero(Var.Val.V, SimplifyQuery(DL, DT, &AC, Var.CxtI))) {
- // Refine MinAbsVarIndex, if abs(Scale*V) >= abs(Scale) holds in the
- // presence of potentially wrapping math.
- if (MultiplyByScaleNoWrap(Var)) {
- // If V != 0 then abs(VarIndex) >= abs(Scale).
- MinAbsVarIndex = Var.Scale.abs();
- }
- }
- } else if (DecompGEP1.VarIndices.size() == 2) {
- // VarIndex = Scale*V0 + (-Scale)*V1.
- // If V0 != V1 then abs(VarIndex) >= abs(Scale).
- // Check that MayBeCrossIteration is false, to avoid reasoning about
- // inequality of values across loop iterations.
- const VariableGEPIndex &Var0 = DecompGEP1.VarIndices[0];
- const VariableGEPIndex &Var1 = DecompGEP1.VarIndices[1];
- if (Var0.hasNegatedScaleOf(Var1) && Var0.Val.TruncBits == 0 &&
- Var0.Val.hasSameCastsAs(Var1.Val) && !AAQI.MayBeCrossIteration &&
- MultiplyByScaleNoWrap(Var0) && MultiplyByScaleNoWrap(Var1) &&
- isKnownNonEqual(Var0.Val.V, Var1.Val.V,
- SimplifyQuery(DL, DT, &AC, /*CxtI=*/Var0.CxtI
- ? Var0.CxtI
- : Var1.CxtI)))
- MinAbsVarIndex = Var0.Scale.abs();
- }
-
- if (MinAbsVarIndex) {
+ // If a minimum absolute variable offset can be established, employ it to
+ // prove that the two accesses are far enough apart.
+ if (auto MinAbsVarIndex =
+ computeMinAbsVarIndexHeuristic(DecompGEP1, DT, AAQI)) {
// The constant offset will have added at least +/-MinAbsVarIndex to it.
APInt OffsetLo = DecompGEP1.Offset - *MinAbsVarIndex;
APInt OffsetHi = DecompGEP1.Offset + *MinAbsVarIndex;
@@ -1427,7 +1335,9 @@ AliasResult BasicAAResult::aliasGEP(
return AliasResult::NoAlias;
}
- if (constantOffsetHeuristic(DecompGEP1, V1Size, V2Size, &AC, DT, AAQI))
+ // As a last attempt, search for a constant offset between the variable
+ // indices that GetLinearExpression could not extract through casts.
+ if (computeConstantOffsetHeuristic(DecompGEP1, V1Size, V2Size, &AC, DT, AAQI))
return AliasResult::NoAlias;
// Statically, we can see that the base objects are the same, but the
@@ -2005,12 +1915,124 @@ void BasicAAResult::subtractDecomposedGEPs(DecomposedGEP &DestGEP,
}
}
-bool BasicAAResult::constantOffsetHeuristic(const DecomposedGEP &GEP,
- LocationSize MaybeV1Size,
- LocationSize MaybeV2Size,
- AssumptionCache *AC,
- DominatorTree *DT,
- const AAQueryInfo &AAQI) {
+BasicAAResult::VariableGEPOffsetInfo
+BasicAAResult::analyzeVariableOffsets(const DecomposedGEP &GEP,
+ DominatorTree *DT) {
+ APInt GCD;
+ ConstantRange OffsetRange(GEP.Offset);
+
+ for (unsigned I = 0, E = GEP.VarIndices.size(); I != E; ++I) {
+ const VariableGEPIndex &Index = GEP.VarIndices[I];
+ const APInt &Scale = Index.Scale;
+
+ SimplifyQuery SQ(DL, DT, &AC, Index.CxtI, /*UseInstrInfo=*/true);
+ KnownBits Known = computeKnownBits(Index.Val.V, SQ);
+
+ APInt ScaleForGCD = Scale;
+ if (!Index.IsNSW)
+ ScaleForGCD =
+ APInt::getOneBitSet(Scale.getBitWidth(), Scale.countr_zero());
+
+ // If V has known trailing zeros, V is a multiple of 2^VarTZ, so
+ // V*Scale is a multiple of ScaleForGCD * 2^VarTZ. Shift ScaleForGCD
+ // left to account for this (trailing zeros compose additively through
+ // multiplication, even in Z/2^n).
+ unsigned VarTZ = Known.countMinTrailingZeros();
+ if (VarTZ > 0) {
+ unsigned MaxShift =
+ Scale.getBitWidth() - ScaleForGCD.getSignificantBits();
+ ScaleForGCD <<= std::min(VarTZ, MaxShift);
+ }
+
+ if (I == 0)
+ GCD = ScaleForGCD.abs();
+ else
+ GCD = APIntOps::GreatestCommonDivisor(GCD, ScaleForGCD.abs());
+
+ ConstantRange CR =
+ computeConstantRange(Index.Val.V, /*ForSigned=*/false, SQ);
+ CR =
+ CR.intersectWith(ConstantRange::fromKnownBits(Known, /*IsSigned=*/true),
+ ConstantRange::Signed);
+ CR = Index.Val.evaluateWith(CR).sextOrTrunc(OffsetRange.getBitWidth());
+
+ assert(OffsetRange.getBitWidth() == Scale.getBitWidth() &&
+ "Bit widths are normalized to MaxIndexSize");
+ if (Index.IsNSW)
+ CR = CR.smul_sat(ConstantRange(Scale));
+ else
+ CR = CR.smul_fast(ConstantRange(Scale));
+
+ if (Index.IsNegated)
+ OffsetRange = OffsetRange.sub(CR);
+ else
+ OffsetRange = OffsetRange.add(CR);
+ }
+
+ return {GCD, OffsetRange};
+}
+
+std::optional<APInt> BasicAAResult::computeMinAbsVarIndexHeuristic(
+ const DecomposedGEP &GEP, DominatorTree *DT, const AAQueryInfo &AAQI) {
+ // Check if abs(V*Scale) >= abs(Scale) holds in the presence of
+ // potentially wrapping math.
+ auto MultiplyByScaleNoWrap = [](const VariableGEPIndex &Var) {
+ if (Var.IsNSW)
+ return true;
+
+ int ValOrigBW = Var.Val.V->getType()->getPrimitiveSizeInBits();
+ // If Scale is small enough so that abs(V*Scale) >= abs(Scale) holds.
+ // The max value of abs(V) is 2^ValOrigBW - 1. Multiplying with a
+ // constant smaller than 2^(bitwidth(Val) - ValOrigBW) won't wrap.
+ int MaxScaleValueBW = Var.Val.getBitWidth() - ValOrigBW;
+ if (MaxScaleValueBW <= 0)
+ return false;
+ return Var.Scale.ule(
+ APInt::getMaxValue(MaxScaleValueBW).zext(Var.Scale.getBitWidth()));
+ };
+
+ const auto &VarIndices = GEP.VarIndices;
+ if (VarIndices.size() == 1) {
+ // VarIndex = Scale*V.
+ const VariableGEPIndex &Var = VarIndices[0];
+ if (Var.Val.TruncBits == 0 &&
+ isKnownNonZero(Var.Val.V, SimplifyQuery(DL, DT, &AC, Var.CxtI))) {
+ // Refine MinAbsVarIndex, if abs(Scale*V) >= abs(Scale) holds in the
+ // presence of potentially wrapping math.
+ if (MultiplyByScaleNoWrap(Var)) {
+ // If V != 0 then abs(VarIndex) >= abs(Scale).
+ return Var.Scale.abs();
+ }
+ }
+ return std::nullopt;
+ }
+
+ if (VarIndices.size() == 2) {
+ // VarIndex = Scale*V0 + (-Scale)*V1.
+ // If V0 != V1 then abs(VarIndex) >= abs(Scale).
+ // Check that MayBeCrossIteration is false, to avoid reasoning about
+ // inequality of values across loop iterations.
+ const VariableGEPIndex &Var0 = VarIndices[0];
+ const VariableGEPIndex &Var1 = VarIndices[1];
+ if (Var0.hasNegatedScaleOf(Var1) && Var0.Val.TruncBits == 0 &&
+ Var0.Val.hasSameCastsAs(Var1.Val) && !AAQI.MayBeCrossIteration &&
+ MultiplyByScaleNoWrap(Var0) && MultiplyByScaleNoWrap(Var1) &&
+ isKnownNonEqual(Var0.Val.V, Var1.Val.V,
+ SimplifyQuery(DL, DT, &AC, /*CxtI=*/Var0.CxtI
+ ? Var0.CxtI
+ : Var1.CxtI)))
+ return Var0.Scale.abs();
+ }
+
+ return std::nullopt;
+}
+
+bool BasicAAResult::computeConstantOffsetHeuristic(const DecomposedGEP &GEP,
+ LocationSize MaybeV1Size,
+ LocationSize MaybeV2Size,
+ AssumptionCache *AC,
+ DominatorTree *DT,
+ const AAQueryInfo &AAQI) {
if (GEP.VarIndices.size() != 2 || !MaybeV1Size.hasValue() ||
!MaybeV2Size.hasValue())
return false;
|
32286a8 to
3ff83c5
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
… (NFC) Part of the offset-based reasoning in `aliasGEP`, including GCD and minimum absolute heuristics, has been abstracted out into `BasicAAResult` private methods, in an attempt to improve code readability. Minor opportunity to modernize code style where possible.
3ff83c5 to
23a814e
Compare
nikic
left a comment
There was a problem hiding this comment.
LGTM, assuming no compile-time impact.
| /// Try to determine the range of values for VarIndex such that | ||
| /// VarIndex <= -MinAbsVarIndex || MinAbsVarIndex <= VarIndex, thus | ||
| /// establishing a minimum absolute value of the variable offset. | ||
| std::optional<APInt> computeMinAbsVarIndexHeuristic(const DecomposedGEP &GEP, |
There was a problem hiding this comment.
| std::optional<APInt> computeMinAbsVarIndexHeuristic(const DecomposedGEP &GEP, | |
| std::optional<APInt> computeMinAbsVarIndex(const DecomposedGEP &GEP, |
Possibly s/Index/Offset as well. I'd expect the function name to describe the meaning of the return value here, which is the minimum absolute value of the variable offset.
Do we need std::optional?
There was a problem hiding this comment.
Sure thing, thanks.
Do we need std::optional?
Right, we could just return a zero offset. However, exceptionally, I think a std::optional here may help make the semantics more explicit (we either proved a bound with a meaningful offset, or we didn't), if that could make sense.
Part of the offset-based reasoning in
aliasGEP, including GCD and minimum absolute heuristics, has been abstracted out intoBasicAAResultprivate methods, in an attempt to improve code readability.Minor opportunity to modernize code style where possible.