[SCEVAA] Enhance SCEVAAResult::alias() to handle two pointers with different pointer bases #91453
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
the
llvm:analysis
|
@llvm/pr-subscribers-llvm-analysis Author: None (csstormq) ChangesIn our out-of-tree target, all memory access are allocated at compile time using Full diff: https://github.com/llvm/llvm-project/pull/91453.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Analysis/ScalarEvolution.h b/llvm/include/llvm/Analysis/ScalarEvolution.h
index 5828cc156cc78..694d2dee9e875 100644
--- a/llvm/include/llvm/Analysis/ScalarEvolution.h
+++ b/llvm/include/llvm/Analysis/ScalarEvolution.h
@@ -721,6 +721,11 @@ class ScalarEvolution {
const SCEV *getTruncateOrSignExtend(const SCEV *V, Type *Ty,
unsigned Depth = 0);
+ /// Return a SCEV corresponding to a conversion of the input value to the
+ /// specified type. If the type must be extended, it is any extended.
+ const SCEV *getTruncateOrAnyExtend(const SCEV *V, Type *Ty,
+ unsigned Depth = 0);
+
/// Return a SCEV corresponding to a conversion of the input value to the
/// specified type. If the type must be extended, it is zero extended. The
/// conversion must not be narrowing.
@@ -754,6 +759,26 @@ class ScalarEvolution {
const SCEV *getUMinFromMismatchedTypes(SmallVectorImpl<const SCEV *> &Ops,
bool Sequential = false);
+ /// Promote the operands to the wider of the types using any-extension, and
+ /// then perform a addrec operation with them.
+ const SCEV *
+ getAddRecExprFromMismatchedTypes(const SmallVectorImpl<const SCEV *> &Ops,
+ const Loop *L, SCEV::NoWrapFlags Flags);
+
+ /// Promote the operands to the wider of the types using any-extension, and
+ /// then perform a add operation with them.
+ const SCEV *
+ getAddExprFromMismatchedTypes(const SmallVectorImpl<const SCEV *> &Ops,
+ SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap,
+ unsigned Depth = 0);
+ const SCEV *
+ getAddExprFromMismatchedTypes(const SCEV *LHS, const SCEV *RHS,
+ SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap,
+ unsigned Depth = 0) {
+ SmallVector<const SCEV *, 2> Ops = {LHS, RHS};
+ return getAddExprFromMismatchedTypes(Ops, Flags, Depth);
+ }
+
/// Transitively follow the chain of pointer-type operands until reaching a
/// SCEV that does not have a single pointer operand. This returns a
/// SCEVUnknown pointer for well-formed pointer-type expressions, but corner
diff --git a/llvm/lib/Analysis/ScalarEvolution.cpp b/llvm/lib/Analysis/ScalarEvolution.cpp
index 93f885c5d5ad8..60bba380f66d6 100644
--- a/llvm/lib/Analysis/ScalarEvolution.cpp
+++ b/llvm/lib/Analysis/ScalarEvolution.cpp
@@ -4650,7 +4650,8 @@ const SCEV *ScalarEvolution::removePointerBase(const SCEV *P) {
Ops[0] = removePointerBase(Ops[0]);
// Don't try to transfer nowrap flags for now. We could in some cases
// (for example, if pointer operand of the AddRec is a SCEVUnknown).
- return getAddRecExpr(Ops, AddRec->getLoop(), SCEV::FlagAnyWrap);
+ return getAddRecExprFromMismatchedTypes(Ops, AddRec->getLoop(),
+ SCEV::FlagAnyWrap);
}
if (auto *Add = dyn_cast<SCEVAddExpr>(P)) {
// The base of an Add is the pointer operand.
@@ -4665,12 +4666,31 @@ const SCEV *ScalarEvolution::removePointerBase(const SCEV *P) {
*PtrOp = removePointerBase(*PtrOp);
// Don't try to transfer nowrap flags for now. We could in some cases
// (for example, if the pointer operand of the Add is a SCEVUnknown).
- return getAddExpr(Ops);
+ return getAddExprFromMismatchedTypes(Ops);
}
+
+ if (auto *Unknown = dyn_cast<SCEVUnknown>(P)) {
+ if (auto *O = dyn_cast<Operator>(Unknown->getValue())) {
+ if (O->getOpcode() == Instruction::IntToPtr) {
+ Value *Op0 = O->getOperand(0);
+ if (isa<ConstantInt>(Op0))
+ return getConstant(dyn_cast<ConstantInt>(Op0));
+ return getSCEV(Op0);
+ }
+ }
+ }
+
// Any other expression must be a pointer base.
return getZero(P->getType());
}
+static bool isIntToPtr(const SCEV *V) {
+ if (auto *Unknown = dyn_cast<SCEVUnknown>(V))
+ if (auto *Op = dyn_cast<Operator>(Unknown->getValue()))
+ return Op->getOpcode() == Instruction::IntToPtr;
+ return false;
+}
+
const SCEV *ScalarEvolution::getMinusSCEV(const SCEV *LHS, const SCEV *RHS,
SCEV::NoWrapFlags Flags,
unsigned Depth) {
@@ -4678,12 +4698,15 @@ const SCEV *ScalarEvolution::getMinusSCEV(const SCEV *LHS, const SCEV *RHS,
if (LHS == RHS)
return getZero(LHS->getType());
- // If we subtract two pointers with different pointer bases, bail.
+ // If we subtract two pointers except inttoptrs with different pointer bases,
+ // bail.
// Eventually, we're going to add an assertion to getMulExpr that we
// can't multiply by a pointer.
if (RHS->getType()->isPointerTy()) {
+ const SCEV *LBase = getPointerBase(LHS);
+ const SCEV *RBase = getPointerBase(RHS);
if (!LHS->getType()->isPointerTy() ||
- getPointerBase(LHS) != getPointerBase(RHS))
+ (LBase != RBase && (!isIntToPtr(LBase) || !isIntToPtr(RBase))))
return getCouldNotCompute();
LHS = removePointerBase(LHS);
RHS = removePointerBase(RHS);
@@ -4718,7 +4741,8 @@ const SCEV *ScalarEvolution::getMinusSCEV(const SCEV *LHS, const SCEV *RHS,
// larger scope than intended.
auto NegFlags = RHSIsNotMinSigned ? SCEV::FlagNSW : SCEV::FlagAnyWrap;
- return getAddExpr(LHS, getNegativeSCEV(RHS, NegFlags), AddFlags, Depth);
+ return getAddExprFromMismatchedTypes(LHS, getNegativeSCEV(RHS, NegFlags),
+ AddFlags, Depth);
}
const SCEV *ScalarEvolution::getTruncateOrZeroExtend(const SCEV *V, Type *Ty,
@@ -4745,6 +4769,18 @@ const SCEV *ScalarEvolution::getTruncateOrSignExtend(const SCEV *V, Type *Ty,
return getSignExtendExpr(V, Ty, Depth);
}
+const SCEV *ScalarEvolution::getTruncateOrAnyExtend(const SCEV *V, Type *Ty,
+ unsigned Depth) {
+ Type *SrcTy = V->getType();
+ assert(SrcTy->isIntOrPtrTy() && Ty->isIntOrPtrTy() &&
+ "Cannot truncate or any extend with non-integer arguments!");
+ if (getTypeSizeInBits(SrcTy) == getTypeSizeInBits(Ty))
+ return V; // No conversion
+ if (getTypeSizeInBits(SrcTy) > getTypeSizeInBits(Ty))
+ return getTruncateExpr(V, Ty, Depth);
+ return getAnyExtendExpr(V, Ty);
+}
+
const SCEV *
ScalarEvolution::getNoopOrZeroExtend(const SCEV *V, Type *Ty) {
Type *SrcTy = V->getType();
@@ -4839,6 +4875,58 @@ ScalarEvolution::getUMinFromMismatchedTypes(SmallVectorImpl<const SCEV *> &Ops,
return getUMinExpr(PromotedOps, Sequential);
}
+const SCEV *ScalarEvolution::getAddRecExprFromMismatchedTypes(
+ const SmallVectorImpl<const SCEV *> &Ops, const Loop *L,
+ SCEV::NoWrapFlags Flags) {
+ assert(!Ops.empty() && "At least one operand must be!");
+ // Trivial case.
+ if (Ops.size() == 1)
+ return Ops[0];
+
+ // Find the max type first.
+ Type *MaxType = nullptr;
+ for (const auto *S : Ops)
+ if (MaxType)
+ MaxType = getWiderType(MaxType, S->getType());
+ else
+ MaxType = S->getType();
+ assert(MaxType && "Failed to find maximum type!");
+
+ // Extend all ops to max type.
+ SmallVector<const SCEV *, 2> PromotedOps;
+ PromotedOps.reserve(Ops.size());
+ for (const auto *S : Ops)
+ PromotedOps.push_back(getNoopOrAnyExtend(S, MaxType));
+
+ return getAddRecExpr(PromotedOps, L, Flags);
+}
+
+const SCEV *ScalarEvolution::getAddExprFromMismatchedTypes(
+ const SmallVectorImpl<const SCEV *> &Ops, SCEV::NoWrapFlags Flags,
+ unsigned Depth) {
+ assert(!Ops.empty() && "At least one operand must be!");
+ // Trivial case.
+ if (Ops.size() == 1)
+ return Ops[0];
+
+ // Find the max type first.
+ Type *MaxType = nullptr;
+ for (const auto *S : Ops)
+ if (MaxType)
+ MaxType = getWiderType(MaxType, S->getType());
+ else
+ MaxType = S->getType();
+ assert(MaxType && "Failed to find maximum type!");
+
+ // Extend all ops to max type.
+ SmallVector<const SCEV *, 2> PromotedOps;
+ PromotedOps.reserve(Ops.size());
+ for (const auto *S : Ops)
+ PromotedOps.push_back(getNoopOrAnyExtend(S, MaxType));
+
+ return getAddExpr(PromotedOps, Flags, Depth);
+}
+
const SCEV *ScalarEvolution::getPointerBase(const SCEV *V) {
// A pointer operand may evaluate to a nonpointer expression, such as null.
if (!V->getType()->isPointerTy())
diff --git a/llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp b/llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
index af8232b03f1ed..c4e13989e17a2 100644
--- a/llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
@@ -67,10 +67,13 @@ AliasResult SCEVAAResult::alias(const MemoryLocation &LocA,
// Test whether the difference is known to be great enough that memory of
// the given sizes don't overlap. This assumes that ASizeInt and BSizeInt
// are non-zero, which is special-cased above.
- if (!isa<SCEVCouldNotCompute>(BA) &&
- ASizeInt.ule(SE.getUnsignedRange(BA).getUnsignedMin()) &&
- (-BSizeInt).uge(SE.getUnsignedRange(BA).getUnsignedMax()))
- return AliasResult::NoAlias;
+ if (!isa<SCEVCouldNotCompute>(BA)) {
+ if (SE.isSCEVable(BA->getType()))
+ BA = SE.getTruncateOrAnyExtend(BA, AS->getType());
+ if (ASizeInt.ule(SE.getUnsignedRange(BA).getUnsignedMin()) &&
+ (-BSizeInt).uge(SE.getUnsignedRange(BA).getUnsignedMax()))
+ return AliasResult::NoAlias;
+ }
// Folding the subtraction while preserving range information can be tricky
// (because of INT_MIN, etc.); if the prior test failed, swap AS and BS
@@ -82,10 +85,13 @@ AliasResult SCEVAAResult::alias(const MemoryLocation &LocA,
// Test whether the difference is known to be great enough that memory of
// the given sizes don't overlap. This assumes that ASizeInt and BSizeInt
// are non-zero, which is special-cased above.
- if (!isa<SCEVCouldNotCompute>(AB) &&
- BSizeInt.ule(SE.getUnsignedRange(AB).getUnsignedMin()) &&
- (-ASizeInt).uge(SE.getUnsignedRange(AB).getUnsignedMax()))
- return AliasResult::NoAlias;
+ if (!isa<SCEVCouldNotCompute>(AB)) {
+ if (SE.isSCEVable(AB->getType()))
+ AB = SE.getTruncateOrAnyExtend(AB, AS->getType());
+ if (BSizeInt.ule(SE.getUnsignedRange(AB).getUnsignedMin()) &&
+ (-ASizeInt).uge(SE.getUnsignedRange(AB).getUnsignedMax()))
+ return AliasResult::NoAlias;
+ }
}
// If ScalarEvolution can find an underlying object, form a new query.
diff --git a/llvm/test/Analysis/ScalarEvolution/scev-aa.ll b/llvm/test/Analysis/ScalarEvolution/scev-aa.ll
index a81baa73a93bd..5610833e9c474 100644
--- a/llvm/test/Analysis/ScalarEvolution/scev-aa.ll
+++ b/llvm/test/Analysis/ScalarEvolution/scev-aa.ll
@@ -340,3 +340,29 @@ for.latch:
for.end:
ret void
}
+
+; CHECK-LABEL: Function: test_different_pointer_bases_of_inttoptr: 2 pointers, 0 call sites
+; CHECK: NoAlias: <16 x i8>* %tmp5, <16 x i8>* %tmp7
+
+define void @test_different_pointer_bases_of_inttoptr() {
+entry:
+ br label %for.body
+
+for.body:
+ %tmp = phi i32 [ %next, %for.body ], [ 1, %entry ]
+ %tmp1 = shl nsw i32 %tmp, 1
+ %tmp2 = add nuw nsw i32 %tmp1, %tmp1
+ %tmp3 = mul nsw i32 %tmp2, 1408
+ %tmp4 = add nsw i32 %tmp3, 1408
+ %tmp5 = getelementptr inbounds i8, ptr inttoptr (i32 1024 to ptr), i32 %tmp1
+ %tmp6 = load <16 x i8>, ptr %tmp5, align 1
+ %tmp7 = getelementptr inbounds i8, ptr inttoptr (i32 4096 to ptr), i32 %tmp4
+ store <16 x i8> %tmp6, ptr %tmp7, align 1
+
+ %next = add i32 %tmp, 2
+ %exitcond = icmp slt i32 %next, 10
+ br i1 %exitcond, label %for.body, label %for.end
+
+for.end:
+ ret void
+}
|
|
See https://reviews.llvm.org/D104806 for why getMinusSCEV works this way. I would rather not relax the current restrictions. What happens if you make SCEVAA explicitly PtrToInt the operands to the subtraction? |
|
Thank you very much, @nikic @efriedma-quic. I have made progress from your comments. |
As you suggested, I have already tried the ptrtpoint pointer operands before calling getMinusSCEV(). |
|
Why was this closed? The patch looks good to me. I'd only suggest to write it in a way that either both pointers are used with ptrtoint or neither, so we can't end up with a ptr + int mix (not entirely sure whether it's possible for just one of the conversions to fail). |
@nikic I'm sorry for that due to misunderstanding your meaning. I am very gald that this patch is beneficial. This pull request has been reopened. Thank you for your feedback. |
…inter bases" This reverts commit 60fd999.
csstormq
changed the title
|
Update the title and description to reflect the intent of the latest code. |
|
@csstormq Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested Please check whether problems have been caused by your change specifically, as How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
In our out-of-tree target, all memory access are allocated at compile time using
inttoptr. We depend onscev-aato break dependencies among these memory locations. However,ScalarEvolution::getMinusSCEV()forbid subracting pointers with different pointer bases. That is, that function always returnsSCEVCouldNotCompute, which does not meet our expectation.
This patch enhances the SCEVAAResult::alias() interface to handle two pointers with different pointer bases.
Before calling getMinusSCEV(), we firstly try to explicitly convert these two pointers into
ptrtointexpressions to do that.
Either both pointers are used with
ptrtointor neither, so we can't end up with a ptr + int mix (not entirely sure whether it's possible for just one of the conversions to fail).