Skip to content

Commit d480f96

Browse files
LebedevRILebedevRI
committed
Revert "[SCEV] Model ashr exact x, C as (abs(x) EXACT/u (1<<C)) * signum(x)"
As being discussed in https://reviews.llvm.org/D100721, this modelling is lossy, we can't reconstruct `ash`/`ashr exact` from it, which means that whenever we actually expand the IR, we've just pessimized the code.. It would be good to model this pattern, after all it comes up every time you want to compute a distance between two pointers, but not at this cost. This reverts commit ec54867.
1 parent d4528cb commit d480f96

File tree

5 files changed

+10
-34
lines changed

5 files changed

+10
-34
lines changed

‎llvm/include/llvm/Analysis/ScalarEvolution.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,6 @@ class ScalarEvolution {
575575
const SCEV *getGEPExpr(GEPOperator *GEP,
576576
const SmallVectorImpl<const SCEV *> &IndexExprs);
577577
const SCEV *getAbsExpr(const SCEV *Op, bool IsNSW);
578-
const SCEV *getSignumExpr(const SCEV *Op);
579578
const SCEV *getMinMaxExpr(SCEVTypes Kind,
580579
SmallVectorImpl<const SCEV *> &Operands);
581580
const SCEV *getSMaxExpr(const SCEV *LHS, const SCEV *RHS);

‎llvm/lib/Analysis/ScalarEvolution.cpp‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3503,11 +3503,6 @@ const SCEV *ScalarEvolution::getAbsExpr(const SCEV *Op, bool IsNSW) {
35033503
return getSMaxExpr(Op, getNegativeSCEV(Op, Flags));
35043504
}
35053505

3506-
const SCEV *ScalarEvolution::getSignumExpr(const SCEV *Op) {
3507-
Type *Ty = Op->getType();
3508-
return getSMinExpr(getSMaxExpr(Op, getMinusOne(Ty)), getOne(Ty));
3509-
}
3510-
35113506
const SCEV *ScalarEvolution::getMinMaxExpr(SCEVTypes Kind,
35123507
SmallVectorImpl<const SCEV *> &Ops) {
35133508
assert(!Ops.empty() && "Cannot get empty (u|s)(min|max)!");
@@ -4559,7 +4554,6 @@ struct BinaryOp {
45594554
Value *RHS;
45604555
bool IsNSW = false;
45614556
bool IsNUW = false;
4562-
bool IsExact = false;
45634557

45644558
/// Op is set if this BinaryOp corresponds to a concrete LLVM instruction or
45654559
/// constant expression.
@@ -4572,14 +4566,11 @@ struct BinaryOp {
45724566
IsNSW = OBO->hasNoSignedWrap();
45734567
IsNUW = OBO->hasNoUnsignedWrap();
45744568
}
4575-
if (auto *PEO = dyn_cast<PossiblyExactOperator>(Op))
4576-
IsExact = PEO->isExact();
45774569
}
45784570

45794571
explicit BinaryOp(unsigned Opcode, Value *LHS, Value *RHS, bool IsNSW = false,
4580-
bool IsNUW = false, bool IsExact = false)
4581-
: Opcode(Opcode), LHS(LHS), RHS(RHS), IsNSW(IsNSW), IsNUW(IsNUW),
4582-
IsExact(IsExact) {}
4572+
bool IsNUW = false)
4573+
: Opcode(Opcode), LHS(LHS), RHS(RHS), IsNSW(IsNSW), IsNUW(IsNUW) {}
45834574
};
45844575

45854576
} // end anonymous namespace
@@ -6745,15 +6736,6 @@ const SCEV *ScalarEvolution::createSCEV(Value *V) {
67456736
}
67466737
}
67476738
}
6748-
if (BO->IsExact) {
6749-
// Given exact arithmetic in-bounds right-shift by a constant,
6750-
// we can lower it into: (abs(x) EXACT/u (1<<C)) * signum(x)
6751-
const SCEV *X = getSCEV(BO->LHS);
6752-
const SCEV *AbsX = getAbsExpr(X, /*IsNSW=*/false);
6753-
APInt Mult = APInt::getOneBitSet(BitWidth, AShrAmt);
6754-
const SCEV *Div = getUDivExactExpr(AbsX, getConstant(Mult));
6755-
return getMulExpr(Div, getSignumExpr(X), SCEV::FlagNSW);
6756-
}
67576739
break;
67586740
}
67596741
}

‎llvm/test/Analysis/ScalarEvolution/ashr.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ define i32 @t3(i32 %x, i32 %y) {
4242
; CHECK-LABEL: 't3'
4343
; CHECK-NEXT: Classifying expressions for: @t3
4444
; CHECK-NEXT: %i0 = ashr exact i32 %x, 4
45-
; CHECK-NEXT: --> ((((-1 * %x) smax %x) /u 16) * (1 smin (-1 smax %x)))<nsw> U: [-268435455,268435456) S: [-268435455,268435456)
45+
; CHECK-NEXT: --> %i0 U: [-134217728,134217728) S: [-134217728,134217728)
4646
; CHECK-NEXT: Determining loop execution counts for: @t3
4747
;
4848
%i0 = ashr exact i32 %x, 4
@@ -65,7 +65,7 @@ define i32 @t5(i32 %x, i32 %y) {
6565
; CHECK-LABEL: 't5'
6666
; CHECK-NEXT: Classifying expressions for: @t5
6767
; CHECK-NEXT: %i0 = ashr exact i32 %x, 5
68-
; CHECK-NEXT: --> ((((-1 * %x) smax %x) /u 32) * (1 smin (-1 smax %x)))<nsw> U: [-134217727,134217728) S: [-134217727,134217728)
68+
; CHECK-NEXT: --> %i0 U: [-67108864,67108864) S: [-67108864,67108864)
6969
; CHECK-NEXT: Determining loop execution counts for: @t5
7070
;
7171
%i0 = ashr exact i32 %x, 5

‎llvm/test/Analysis/ScalarEvolution/ptrtoint.ll‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,9 +473,9 @@ define void @pr46786_c26_int(i32* %arg, i32* %arg1, i32* %arg2) {
473473
; X64-NEXT: %i10 = sub i64 %i9, %i4
474474
; X64-NEXT: --> {0,+,4}<nw><%bb6> U: [0,-3) S: [-9223372036854775808,9223372036854775805) Exits: (4 * ((-4 + (-1 * %arg) + %arg1) /u 4))<nuw> LoopDispositions: { %bb6: Computable }
475475
; X64-NEXT: %i11 = ashr exact i64 %i10, 2
476-
; X64-NEXT: --> ((({0,+,4}<nw><%bb6> smax {0,+,-4}<nw><%bb6>) /u 4) * (1 smin (-1 smax {0,+,4}<nw><%bb6>)))<nsw> U: [-4611686018427387903,4611686018427387904) S: [-4611686018427387903,4611686018427387904) Exits: ((((4 * ((-4 + (-1 * %arg) + %arg1) /u 4))<nuw> smax (-4 * ((-4 + (-1 * %arg) + %arg1) /u 4))) /u 4) * (1 smin (-1 smax (4 * ((-4 + (-1 * %arg) + %arg1) /u 4))<nuw>)))<nsw> LoopDispositions: { %bb6: Computable }
476+
; X64-NEXT: --> %i11 U: [-2305843009213693952,2305843009213693952) S: [-2305843009213693952,2305843009213693952) Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
477477
; X64-NEXT: %i12 = getelementptr inbounds i32, i32* %arg2, i64 %i11
478-
; X64-NEXT: --> ((4 * (({0,+,4}<nw><%bb6> smax {0,+,-4}<nw><%bb6>) /u 4) * (1 smin (-1 smax {0,+,4}<nw><%bb6>))) + %arg2) U: full-set S: full-set Exits: ((4 * (((4 * ((-4 + (-1 * %arg) + %arg1) /u 4))<nuw> smax (-4 * ((-4 + (-1 * %arg) + %arg1) /u 4))) /u 4) * (1 smin (-1 smax (4 * ((-4 + (-1 * %arg) + %arg1) /u 4))<nuw>))) + %arg2) LoopDispositions: { %bb6: Computable }
478+
; X64-NEXT: --> ((4 * %i11)<nsw> + %arg2) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
479479
; X64-NEXT: %i13 = load i32, i32* %i12, align 4
480480
; X64-NEXT: --> %i13 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
481481
; X64-NEXT: %i14 = add nsw i32 %i13, %i8
@@ -502,9 +502,9 @@ define void @pr46786_c26_int(i32* %arg, i32* %arg1, i32* %arg2) {
502502
; X32-NEXT: %i10 = sub i64 %i9, %i4
503503
; X32-NEXT: --> {0,+,4}<nw><%bb6> U: [0,4294967293) S: [0,4294967293) Exits: (4 * ((zext i32* (-4 + (-1 * %arg) + %arg1) to i64) /u 4))<nuw><nsw> LoopDispositions: { %bb6: Computable }
504504
; X32-NEXT: %i11 = ashr exact i64 %i10, 2
505-
; X32-NEXT: --> ({0,+,1}<nw><%bb6> * (1 smin {0,+,4}<nuw><nsw><%bb6>))<nuw><nsw> U: [0,1073741824) S: [0,1073741824) Exits: (((zext i32* (-4 + (-1 * %arg) + %arg1) to i64) /u 4) * (1 smin (4 * ((zext i32* (-4 + (-1 * %arg) + %arg1) to i64) /u 4))<nuw><nsw>))<nuw><nsw> LoopDispositions: { %bb6: Computable }
505+
; X32-NEXT: --> %i11 U: [-2147483648,2147483648) S: [-2147483648,2147483648) Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
506506
; X32-NEXT: %i12 = getelementptr inbounds i32, i32* %arg2, i64 %i11
507-
; X32-NEXT: --> (((trunc i64 (1 smin {0,+,4}<nuw><nsw><%bb6>) to i32) * {0,+,4}<%bb6>) + %arg2) U: full-set S: full-set Exits: ((4 * (trunc i64 (1 smin (4 * ((zext i32* (-4 + (-1 * %arg) + %arg1) to i64) /u 4))<nuw><nsw>) to i32) * ((-4 + (-1 * %arg) + %arg1) /u 4)) + %arg2) LoopDispositions: { %bb6: Computable }
507+
; X32-NEXT: --> ((4 * (trunc i64 %i11 to i32))<nsw> + %arg2) U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
508508
; X32-NEXT: %i13 = load i32, i32* %i12, align 4
509509
; X32-NEXT: --> %i13 U: full-set S: full-set Exits: <<Unknown>> LoopDispositions: { %bb6: Variant }
510510
; X32-NEXT: %i14 = add nsw i32 %i13, %i8

‎llvm/test/Transforms/IndVarSimplify/ashr-expansion.ll‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,8 @@ target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
66
define float @ashr_expansion_valid(i64 %x, float* %ptr) {
77
; CHECK-LABEL: @ashr_expansion_valid(
88
; CHECK-NEXT: entry:
9-
; CHECK-NEXT: [[SMAX:%.*]] = call i64 @llvm.smax.i64(i64 [[X:%.*]], i64 -1)
10-
; CHECK-NEXT: [[SMIN:%.*]] = call i64 @llvm.smin.i64(i64 [[SMAX]], i64 1)
11-
; CHECK-NEXT: [[TMP0:%.*]] = sub i64 0, [[X]]
12-
; CHECK-NEXT: [[SMAX1:%.*]] = call i64 @llvm.smax.i64(i64 [[X]], i64 [[TMP0]])
13-
; CHECK-NEXT: [[TMP1:%.*]] = lshr i64 [[SMAX1]], 4
14-
; CHECK-NEXT: [[TMP2:%.*]] = mul nsw i64 [[SMIN]], [[TMP1]]
15-
; CHECK-NEXT: [[UMAX:%.*]] = call i64 @llvm.umax.i64(i64 [[TMP2]], i64 1)
9+
; CHECK-NEXT: [[BOUND:%.*]] = ashr exact i64 [[X:%.*]], 4
10+
; CHECK-NEXT: [[UMAX:%.*]] = call i64 @llvm.umax.i64(i64 [[BOUND]], i64 1)
1611
; CHECK-NEXT: br label [[LOOP:%.*]]
1712
; CHECK: loop:
1813
; CHECK-NEXT: [[IV:%.*]] = phi i64 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]

0 commit comments

Comments
 (0)