Commit d251e94e authored by Hans Wennborg's avatar Hans Wennborg
Browse files

Merging r296992: 

------------------------------------------------------------------------
r296992 | sanjoy | 2017-03-05 15:49:17 -0800 (Sun, 05 Mar 2017) | 7 lines

[SCEV] Decrease the recursion threshold for CompareValueComplexity

Fixes PR32142.

r287232 accidentally increased the recursion threshold for
CompareValueComplexity from 2 to 32.  This change reverses that change
by introducing a separate flag for CompareValueComplexity's threshold.
------------------------------------------------------------------------

llvm-svn: 297164
parent ed6aea83

llvm/lib/Analysis/ScalarEvolution.cpp

+11 −6
Original line number Diff line number Diff line
@@ -127,11 +127,16 @@ static cl::opt<unsigned> MulOpsInlineThreshold( 
    cl::desc("Threshold for inlining multiplication operands into a SCEV"),

    cl::init(1000));



static cl::opt<unsigned>

    MaxCompareDepth("scalar-evolution-max-compare-depth", cl::Hidden,

                    cl::desc("Maximum depth of recursive compare complexity"),

static cl::opt<unsigned> MaxSCEVCompareDepth(

    "scalar-evolution-max-scev-compare-depth", cl::Hidden,

    cl::desc("Maximum depth of recursive SCEV complexity comparisons"),

    cl::init(32));



static cl::opt<unsigned> MaxValueCompareDepth(

    "scalar-evolution-max-value-compare-depth", cl::Hidden,

    cl::desc("Maximum depth of recursive value complexity comparisons"),

    cl::init(2));



//===----------------------------------------------------------------------===//

//                           SCEV class definitions

//===----------------------------------------------------------------------===//
@@ -481,7 +486,7 @@ static int 
CompareValueComplexity(SmallSet<std::pair<Value *, Value *>, 8> &EqCache,

                       const LoopInfo *const LI, Value *LV, Value *RV,

                       unsigned Depth) {

  if (Depth > MaxCompareDepth || EqCache.count({LV, RV}))

  if (Depth > MaxValueCompareDepth || EqCache.count({LV, RV}))

    return 0;



  // Order pointer values after integer values. This helps SCEVExpander form
@@ -568,7 +573,7 @@ static int CompareSCEVComplexity( 
  if (LType != RType)

    return (int)LType - (int)RType;



  if (Depth > MaxCompareDepth || EqCacheSCEV.count({LHS, RHS}))

  if (Depth > MaxSCEVCompareDepth || EqCacheSCEV.count({LHS, RHS}))

    return 0;

  // Aside from the getSCEVType() ordering, the particular ordering

  // isn't very important except that it's beneficial to be consistent,

llvm/unittests/Analysis/ScalarEvolutionTest.cpp

+37 −1
Original line number Diff line number Diff line
@@ -465,7 +465,7 @@ TEST_F(ScalarEvolutionsTest, CommutativeExprOperandOrder) { 
    });

}



TEST_F(ScalarEvolutionsTest, SCEVCompareComplexity) {

TEST_F(ScalarEvolutionsTest, CompareSCEVComplexity) {

  FunctionType *FTy =

      FunctionType::get(Type::getVoidTy(Context), std::vector<Type *>(), false);

  Function *F = cast<Function>(M.getOrInsertFunction("f", FTy));
@@ -532,5 +532,41 @@ TEST_F(ScalarEvolutionsTest, SCEVCompareComplexity) { 
  EXPECT_NE(nullptr, SE.getSCEV(Acc[0]));

}



TEST_F(ScalarEvolutionsTest, CompareValueComplexity) {

  IntegerType *IntPtrTy = M.getDataLayout().getIntPtrType(Context);

  PointerType *IntPtrPtrTy = IntPtrTy->getPointerTo();



  FunctionType *FTy =

      FunctionType::get(Type::getVoidTy(Context), {IntPtrTy, IntPtrTy}, false);

  Function *F = cast<Function>(M.getOrInsertFunction("f", FTy));

  BasicBlock *EntryBB = BasicBlock::Create(Context, "entry", F);



  Value *X = &*F->arg_begin();

  Value *Y = &*std::next(F->arg_begin());



  const int ValueDepth = 10;

  for (int i = 0; i < ValueDepth; i++) {

    X = new LoadInst(new IntToPtrInst(X, IntPtrPtrTy, "", EntryBB), "",

                     /*isVolatile*/ false, EntryBB);

    Y = new LoadInst(new IntToPtrInst(Y, IntPtrPtrTy, "", EntryBB), "",

                     /*isVolatile*/ false, EntryBB);

  }



  auto *MulA = BinaryOperator::CreateMul(X, Y, "", EntryBB);

  auto *MulB = BinaryOperator::CreateMul(Y, X, "", EntryBB);

  ReturnInst::Create(Context, nullptr, EntryBB);



  // This test isn't checking for correctness.  Today making A and B resolve to

  // the same SCEV would require deeper searching in CompareValueComplexity,

  // which will slow down compilation.  However, this test can fail (with LLVM's

  // behavior still being correct) if we ever have a smarter

  // CompareValueComplexity that is both fast and more accurate.



  ScalarEvolution SE = buildSE(*F);

  auto *A = SE.getSCEV(MulA);

  auto *B = SE.getSCEV(MulB);

  EXPECT_NE(A, B);

}



}  // end anonymous namespace

}  // end namespace llvm