Skip to content

Commit

Permalink
[ScalarEvolution] Strictly enforce pointer/int type rules.
Browse files Browse the repository at this point in the history 
Rules:

1. SCEVUnknown is a pointer if and only if the LLVM IR value is a
   pointer.
2. SCEVPtrToInt is never a pointer.
3. If any other SCEV expression has no pointer operands, the result is
   an integer.
4. If a SCEVAddExpr has exactly one pointer operand, the result is a
   pointer.
5. If a SCEVAddRecExpr's first operand is a pointer, and it has no other
   pointer operands, the result is a pointer.
6. If every operand of a SCEVMinMaxExpr is a pointer, the result is a
   pointer.
7. Otherwise, the SCEV expression is invalid.

I'm not sure how useful rule 6 is in practice.  If we exclude it, we can
guarantee that ScalarEvolution::getPointerBase always returns a
SCEVUnknown, which might be a helpful property. Anyway, I'll leave that
for a followup.

This is basically mop-up at this point; all the changes with significant
functional effects have landed.  Some of the remaining changes could be
split off, but I don't see much point.

Differential Revision: https://reviews.llvm.org/D105510
  • Loading branch information
@efriedma-quic
efriedma-quic committed Jul 10, 2021
1 parent 8e9216f commit 9c4baf5
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 21 deletions.
38 changes: 30 additions & 8 deletions llvm/lib/Analysis/ScalarEvolution.cpp
View file
Expand Up @@ -1192,6 +1192,7 @@ const SCEV *ScalarEvolution::getTruncateExpr(const SCEV *Op, Type *Ty,
"This is not a truncating conversion!");
assert(isSCEVable(Ty) &&
"This is not a conversion to a SCEVable type!");
assert(!Op->getType()->isPointerTy() && "Can't truncate pointer!");
Ty = getEffectiveSCEVType(Ty);

FoldingSetNodeID ID;
Expand Down Expand Up @@ -1581,6 +1582,7 @@ ScalarEvolution::getZeroExtendExpr(const SCEV *Op, Type *Ty, unsigned Depth) {
"This is not an extending conversion!");
assert(isSCEVable(Ty) &&
"This is not a conversion to a SCEVable type!");
assert(!Op->getType()->isPointerTy() && "Can't extend pointer!");
Ty = getEffectiveSCEVType(Ty);

// Fold if the operand is constant.
Expand Down Expand Up @@ -1883,6 +1885,7 @@ ScalarEvolution::getSignExtendExpr(const SCEV *Op, Type *Ty, unsigned Depth) {
"This is not an extending conversion!");
assert(isSCEVable(Ty) &&
"This is not a conversion to a SCEVable type!");
assert(!Op->getType()->isPointerTy() && "Can't extend pointer!");
Ty = getEffectiveSCEVType(Ty);

// Fold if the operand is constant.
Expand Down Expand Up @@ -2410,6 +2413,9 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
for (unsigned i = 1, e = Ops.size(); i != e; ++i)
assert(getEffectiveSCEVType(Ops[i]->getType()) == ETy &&
"SCEVAddExpr operand types don't match!");
unsigned NumPtrs = count_if(
Ops, [](const SCEV *Op) { return Op->getType()->isPointerTy(); });
assert(NumPtrs <= 1 && "add has at most one pointer operand");
#endif

// Sort by complexity, this groups all similar expression types together.
Expand Down Expand Up @@ -2645,12 +2651,16 @@ const SCEV *ScalarEvolution::getAddExpr(SmallVectorImpl<const SCEV *> &Ops,
Ops.clear();
if (AccumulatedConstant != 0)
Ops.push_back(getConstant(AccumulatedConstant));
for (auto &MulOp : MulOpLists)
if (MulOp.first != 0)
for (auto &MulOp : MulOpLists) {
if (MulOp.first == 1) {
Ops.push_back(getAddExpr(MulOp.second, SCEV::FlagAnyWrap, Depth + 1));
} else if (MulOp.first != 0) {
Ops.push_back(getMulExpr(
getConstant(MulOp.first),
getAddExpr(MulOp.second, SCEV::FlagAnyWrap, Depth + 1),
SCEV::FlagAnyWrap, Depth + 1));
}
}
if (Ops.empty())
return getZero(Ty);
if (Ops.size() == 1)
Expand Down Expand Up @@ -2969,9 +2979,10 @@ const SCEV *ScalarEvolution::getMulExpr(SmallVectorImpl<const SCEV *> &Ops,
assert(!Ops.empty() && "Cannot get empty mul!");
if (Ops.size() == 1) return Ops[0];
#ifndef NDEBUG
Type *ETy = getEffectiveSCEVType(Ops[0]->getType());
Type *ETy = Ops[0]->getType();
assert(!ETy->isPointerTy());
for (unsigned i = 1, e = Ops.size(); i != e; ++i)
assert(getEffectiveSCEVType(Ops[i]->getType()) == ETy &&
assert(Ops[i]->getType() == ETy &&
"SCEVMulExpr operand types don't match!");
#endif

Expand Down Expand Up @@ -3256,8 +3267,9 @@ const SCEV *ScalarEvolution::getURemExpr(const SCEV *LHS,
/// possible.
const SCEV *ScalarEvolution::getUDivExpr(const SCEV *LHS,
const SCEV *RHS) {
assert(getEffectiveSCEVType(LHS->getType()) ==
getEffectiveSCEVType(RHS->getType()) &&
assert(!LHS->getType()->isPointerTy() &&
"SCEVUDivExpr operand can't be pointer!");
assert(LHS->getType() == RHS->getType() &&
"SCEVUDivExpr operand types don't match!");

FoldingSetNodeID ID;
Expand Down Expand Up @@ -3506,9 +3518,11 @@ ScalarEvolution::getAddRecExpr(SmallVectorImpl<const SCEV *> &Operands,
if (Operands.size() == 1) return Operands[0];
#ifndef NDEBUG
Type *ETy = getEffectiveSCEVType(Operands[0]->getType());
for (unsigned i = 1, e = Operands.size(); i != e; ++i)
for (unsigned i = 1, e = Operands.size(); i != e; ++i) {
assert(getEffectiveSCEVType(Operands[i]->getType()) == ETy &&
"SCEVAddRecExpr operand types don't match!");
assert(!Operands[i]->getType()->isPointerTy() && "Step must be integer");
}
for (unsigned i = 0, e = Operands.size(); i != e; ++i)
assert(isLoopInvariant(Operands[i], L) &&
"SCEVAddRecExpr operand is not loop-invariant!");
Expand Down Expand Up @@ -3662,9 +3676,13 @@ const SCEV *ScalarEvolution::getMinMaxExpr(SCEVTypes Kind,
if (Ops.size() == 1) return Ops[0];
#ifndef NDEBUG
Type *ETy = getEffectiveSCEVType(Ops[0]->getType());
for (unsigned i = 1, e = Ops.size(); i != e; ++i)
for (unsigned i = 1, e = Ops.size(); i != e; ++i) {
assert(getEffectiveSCEVType(Ops[i]->getType()) == ETy &&
"Operand types don't match!");
assert(Ops[0]->getType()->isPointerTy() ==
Ops[i]->getType()->isPointerTy() &&
"min/max should be consistently pointerish");
}
#endif

bool IsSigned = Kind == scSMaxExpr || Kind == scSMinExpr;
Expand Down Expand Up @@ -10579,6 +10597,8 @@ bool ScalarEvolution::isImpliedCond(ICmpInst::Predicate Pred, const SCEV *LHS,
}
}

if (LHS->getType()->isPointerTy())
return false;
if (CmpInst::isSigned(Pred)) {
LHS = getSignExtendExpr(LHS, FoundLHS->getType());
RHS = getSignExtendExpr(RHS, FoundLHS->getType());
Expand All @@ -10588,6 +10608,8 @@ bool ScalarEvolution::isImpliedCond(ICmpInst::Predicate Pred, const SCEV *LHS,
}
} else if (getTypeSizeInBits(LHS->getType()) >
getTypeSizeInBits(FoundLHS->getType())) {
if (FoundLHS->getType()->isPointerTy())
return false;
if (CmpInst::isSigned(FoundPred)) {
FoundLHS = getSignExtendExpr(FoundLHS, LHS->getType());
FoundRHS = getSignExtendExpr(FoundRHS, LHS->getType());
Expand Down
10 changes: 7 additions & 3 deletions llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
View file
Expand Up @@ -682,8 +682,11 @@ static const SCEV *getExactSDiv(const SCEV *LHS, const SCEV *RHS,
const APInt &RA = RC->getAPInt();
// Handle x /s -1 as x * -1, to give ScalarEvolution a chance to do
// some folding.
if (RA.isAllOnesValue())
if (RA.isAllOnesValue()) {
if (LHS->getType()->isPointerTy())
return nullptr;
return SE.getMulExpr(LHS, RC);
}
// Handle x /s 1 as x.
if (RA == 1)
return LHS;
Expand Down Expand Up @@ -4063,7 +4066,8 @@ void LSRInstance::GenerateTruncates(LSRUse &LU, unsigned LUIdx, Formula Base) {
// Determine the integer type for the base formula.
Type *DstTy = Base.getType();
if (!DstTy) return;
DstTy = SE.getEffectiveSCEVType(DstTy);
if (DstTy->isPointerTy())
return;

for (Type *SrcTy : Types) {
if (SrcTy != DstTy && TTI.isTruncateFree(SrcTy, DstTy)) {
Expand Down Expand Up @@ -5301,7 +5305,7 @@ Value *LSRInstance::Expand(const LSRUse &LU, const LSRFixup &LF,
if (F.BaseGV) {
// Flush the operand list to suppress SCEVExpander hoisting.
if (!Ops.empty()) {
Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), Ty);
Value *FullV = Rewriter.expandCodeFor(SE.getAddExpr(Ops), IntTy);
Ops.clear();
Ops.push_back(SE.getUnknown(FullV));
}
Expand Down
11 changes: 7 additions & 4 deletions llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
View file
Expand Up @@ -1147,6 +1147,10 @@ static bool canBeCheaplyTransformed(ScalarEvolution &SE,
const SCEVAddRecExpr *Phi,
const SCEVAddRecExpr *Requested,
bool &InvertStep) {
// We can't transform to match a pointer PHI.
if (Phi->getType()->isPointerTy())
return false;

Type *PhiTy = SE.getEffectiveSCEVType(Phi->getType());
Type *RequestedTy = SE.getEffectiveSCEVType(Requested->getType());

Expand All @@ -1165,8 +1169,7 @@ static bool canBeCheaplyTransformed(ScalarEvolution &SE,
}

// Check whether inverting will help: {R,+,-1} == R - {0,+,1}.
if (SE.getAddExpr(Requested->getStart(),
SE.getNegativeSCEV(Requested)) == Phi) {
if (SE.getMinusSCEV(Requested->getStart(), Requested) == Phi) {
InvertStep = true;
return true;
}
Expand Down Expand Up @@ -1577,8 +1580,8 @@ Value *SCEVExpander::visitAddRecExpr(const SCEVAddRecExpr *S) {
// Rewrite an AddRec in terms of the canonical induction variable, if
// its type is more narrow.
if (CanonicalIV &&
SE.getTypeSizeInBits(CanonicalIV->getType()) >
SE.getTypeSizeInBits(Ty)) {
SE.getTypeSizeInBits(CanonicalIV->getType()) > SE.getTypeSizeInBits(Ty) &&
!S->getType()->isPointerTy()) {
SmallVector<const SCEV *, 4> NewOps(S->getNumOperands());
for (unsigned i = 0, e = S->getNumOperands(); i != e; ++i)
NewOps[i] = SE.getAnyExtendExpr(S->op_begin()[i], CanonicalIV->getType());
Expand Down
13 changes: 7 additions & 6 deletions llvm/unittests/Analysis/ScalarEvolutionTest.cpp
View file
Expand Up @@ -96,13 +96,13 @@ TEST_F(ScalarEvolutionsTest, SCEVUnknownRAUW) {
const SCEV *S1 = SE.getSCEV(V1);
const SCEV *S2 = SE.getSCEV(V2);

const SCEV *P0 = SE.getAddExpr(S0, S0);
const SCEV *P1 = SE.getAddExpr(S1, S1);
const SCEV *P2 = SE.getAddExpr(S2, S2);
const SCEV *P0 = SE.getAddExpr(S0, SE.getConstant(S0->getType(), 2));
const SCEV *P1 = SE.getAddExpr(S1, SE.getConstant(S0->getType(), 2));
const SCEV *P2 = SE.getAddExpr(S2, SE.getConstant(S0->getType(), 2));

const SCEVMulExpr *M0 = cast<SCEVMulExpr>(P0);
const SCEVMulExpr *M1 = cast<SCEVMulExpr>(P1);
const SCEVMulExpr *M2 = cast<SCEVMulExpr>(P2);
auto *M0 = cast<SCEVAddExpr>(P0);
auto *M1 = cast<SCEVAddExpr>(P1);
auto *M2 = cast<SCEVAddExpr>(P2);

EXPECT_EQ(cast<SCEVConstant>(M0->getOperand(0))->getValue()->getZExtValue(),
2u);
Expand Down Expand Up @@ -707,6 +707,7 @@ TEST_F(ScalarEvolutionsTest, SCEVZeroExtendExpr) {
ReturnInst::Create(Context, nullptr, EndBB);
ScalarEvolution SE = buildSE(*F);
const SCEV *S = SE.getSCEV(Accum);
S = SE.getLosslessPtrToIntExpr(S);
Type *I128Ty = Type::getInt128Ty(Context);
SE.getZeroExtendExpr(S, I128Ty);
}
Expand Down

0 comments on commit 9c4baf5

Please sign in to comment.