Skip to content

Commit 1fb6104

Browse files
committed
Reland "[SCEV] Model ptrtoint(SCEVUnknown) cast not as unknown, but as zext/trunc/self of SCEVUnknown"
This relands commit 1c021c6 which was reverted in commit 17cec6a because an assertion was being triggered, since `BuildConstantFromSCEV()` wasn't updated to handle the case where the constant we want to truncate is actually a pointer. I was unsuccessful in coming up with a test case where we'd end there with constant zext/sext of a pointer, so i didn't handle those cases there until there is a test case. Original commit message: While we indeed can't treat them as no-ops, i believe we can/should do better than just modelling them as `unknown`. `inttoptr` story is complicated, but for `ptrtoint`, it seems straight-forward to model it just as a zext-or-trunc of unknown. This may be important now that we track towards making inttoptr/ptrtoint casts not no-op, and towards preventing folding them into loads/etc (see D88979/D88789/D88788) Reviewed By: mkazantsev Differential Revision: https://reviews.llvm.org/D88806
1 parent 73818f4 commit 1fb6104

File tree

10 files changed

+140
-137
lines changed

10 files changed

+140
-137
lines changed

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3505,15 +3505,15 @@ const SCEV *ScalarEvolution::getUMinExpr(SmallVectorImpl<const SCEV *> &Ops) {
35053505
}
35063506

35073507
const SCEV *ScalarEvolution::getSizeOfExpr(Type *IntTy, Type *AllocTy) {
3508-
// We can bypass creating a target-independent
3509-
// constant expression and then folding it back into a ConstantInt.
3510-
// This is just a compile-time optimization.
35113508
if (isa<ScalableVectorType>(AllocTy)) {
35123509
Constant *NullPtr = Constant::getNullValue(AllocTy->getPointerTo());
35133510
Constant *One = ConstantInt::get(IntTy, 1);
35143511
Constant *GEP = ConstantExpr::getGetElementPtr(AllocTy, NullPtr, One);
3515-
return getSCEV(ConstantExpr::getPtrToInt(GEP, IntTy));
3512+
return getUnknown(ConstantExpr::getPtrToInt(GEP, IntTy));
35163513
}
3514+
// We can bypass creating a target-independent
3515+
// constant expression and then folding it back into a ConstantInt.
3516+
// This is just a compile-time optimization.
35173517
return getConstant(IntTy, getDataLayout().getTypeAllocSize(AllocTy));
35183518
}
35193519

@@ -6301,6 +6301,36 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) {
63016301
return getSCEV(U->getOperand(0));
63026302
break;
63036303

6304+
case Instruction::PtrToInt: {
6305+
// It's tempting to handle inttoptr and ptrtoint as no-ops,
6306+
// however this can lead to pointer expressions which cannot safely be
6307+
// expanded to GEPs because ScalarEvolution doesn't respect
6308+
// the GEP aliasing rules when simplifying integer expressions.
6309+
//
6310+
// However, given
6311+
// %x = ???
6312+
// %y = ptrtoint %x
6313+
// %z = ptrtoint %x
6314+
// it is safe to say that %y and %z are the same thing.
6315+
//
6316+
// So instead of modelling the cast itself as unknown,
6317+
// since the casts are transparent within SCEV,
6318+
// we can at least model the casts original value as unknow instead.
6319+
6320+
// BUT, there's caveat. If we simply model %x as unknown, unrelated uses
6321+
// of %x will also see it as unknown, which is obviously bad.
6322+
// So we can only do this iff %x would be modelled as unknown anyways.
6323+
auto *OpSCEV = getSCEV(U->getOperand(0));
6324+
if (isa<SCEVUnknown>(OpSCEV))
6325+
return getTruncateOrZeroExtend(OpSCEV, U->getType());
6326+
// If we can model the operand, however, we must fallback to modelling
6327+
// the whole cast as unknown instead.
6328+
LLVM_FALLTHROUGH;
6329+
}
6330+
case Instruction::IntToPtr:
6331+
// We can't do this for inttoptr at all, however.
6332+
return getUnknown(V);
6333+
63046334
case Instruction::SDiv:
63056335
// If both operands are non-negative, this is just an udiv.
63066336
if (isKnownNonNegative(getSCEV(U->getOperand(0))) &&
@@ -6315,11 +6345,6 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) {
63156345
return getURemExpr(getSCEV(U->getOperand(0)), getSCEV(U->getOperand(1)));
63166346
break;
63176347

6318-
// It's tempting to handle inttoptr and ptrtoint as no-ops, however this can
6319-
// lead to pointer expressions which cannot safely be expanded to GEPs,
6320-
// because ScalarEvolution doesn't respect the GEP aliasing rules when
6321-
// simplifying integer expressions.
6322-
63236348
case Instruction::GetElementPtr:
63246349
return createNodeForGEP(cast<GEPOperator>(U));
63256350

@@ -7974,8 +7999,11 @@ static Constant *BuildConstantFromSCEV(const SCEV *V) {
79747999
}
79758000
case scTruncate: {
79768001
const SCEVTruncateExpr *ST = cast<SCEVTruncateExpr>(V);
7977-
if (Constant *CastOp = BuildConstantFromSCEV(ST->getOperand()))
7978-
return ConstantExpr::getTrunc(CastOp, ST->getType());
8002+
if (Constant *CastOp = BuildConstantFromSCEV(ST->getOperand())) {
8003+
if (!CastOp->getType()->isPointerTy())
8004+
return ConstantExpr::getTrunc(CastOp, ST->getType());
8005+
return ConstantExpr::getPtrToInt(CastOp, ST->getType());
8006+
}
79798007
break;
79808008
}
79818009
case scAddExpr: {

llvm/lib/Transforms/Utils/SimplifyIndVar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ static bool willNotOverflow(ScalarEvolution *SE, Instruction::BinaryOps BinOp,
427427
: &ScalarEvolution::getZeroExtendExpr;
428428

429429
// Check ext(LHS op RHS) == ext(LHS) op ext(RHS)
430-
auto *NarrowTy = cast<IntegerType>(LHS->getType());
430+
auto *NarrowTy = cast<IntegerType>(SE->getEffectiveSCEVType(LHS->getType()));
431431
auto *WideTy =
432432
IntegerType::get(NarrowTy->getContext(), NarrowTy->getBitWidth() * 2);
433433

llvm/test/Analysis/ScalarEvolution/add-expr-pointer-operand-sorting.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ define i32 @d(i32 %base) {
3333
; CHECK-NEXT: %1 = load i32*, i32** @c, align 8
3434
; CHECK-NEXT: --> %1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
3535
; CHECK-NEXT: %sub.ptr.lhs.cast = ptrtoint i32* %1 to i64
36-
; CHECK-NEXT: --> %sub.ptr.lhs.cast U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
36+
; CHECK-NEXT: --> %1 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
3737
; CHECK-NEXT: %sub.ptr.sub = sub i64 %sub.ptr.lhs.cast, ptrtoint ([1 x i32]* @b to i64)
38-
; CHECK-NEXT: --> ((-1 * ptrtoint ([1 x i32]* @b to i64)) + %sub.ptr.lhs.cast) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
38+
; CHECK-NEXT: --> ((-1 * @b) + %1) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
3939
; CHECK-NEXT: %sub.ptr.div = sdiv exact i64 %sub.ptr.sub, 4
4040
; CHECK-NEXT: --> %sub.ptr.div U: full-set S: [-2305843009213693952,2305843009213693952) Exits: <<Unknown>> LoopDispositions: { %for.cond: Variant }
4141
; CHECK-NEXT: %arrayidx1 = getelementptr inbounds [1 x i8], [1 x i8]* %arrayidx, i64 0, i64 %sub.ptr.div

llvm/test/Analysis/ScalarEvolution/no-wrap-add-exprs.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,14 +170,14 @@ define void @f3(i8* %x_addr, i8* %y_addr, i32* %tmp_addr) {
170170
%int5 = add i32 %int0, 5
171171
%int.zext = zext i32 %int5 to i64
172172
; CHECK: %int.zext = zext i32 %int5 to i64
173-
; CHECK-NEXT: --> (1 + (zext i32 (4 + %int0) to i64))<nuw><nsw> U: [1,4294967294) S: [1,4294967297)
173+
; CHECK-NEXT: --> (1 + (zext i32 (4 + (trunc [16 x i8]* @z_addr to i32)) to i64))<nuw><nsw> U: [1,4294967294) S: [1,4294967297)
174174

175175
%ptr_noalign = bitcast [16 x i8]* @z_addr_noalign to i8*
176176
%int0_na = ptrtoint i8* %ptr_noalign to i32
177177
%int5_na = add i32 %int0_na, 5
178178
%int.zext_na = zext i32 %int5_na to i64
179179
; CHECK: %int.zext_na = zext i32 %int5_na to i64
180-
; CHECK-NEXT: --> (zext i32 (5 + %int0_na) to i64) U: [0,4294967296) S: [0,4294967296)
180+
; CHECK-NEXT: --> (zext i32 (5 + (trunc [16 x i8]* @z_addr_noalign to i32)) to i64) U: [0,4294967296) S: [0,4294967296)
181181

182182
%tmp = load i32, i32* %tmp_addr
183183
%mul = and i32 %tmp, -4

0 commit comments

Comments
 (0)