[llvm] r351725 - [SCEV][NFC] Introduces expression sizes estimation
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 21 00:59:01 PST 2019
On 1/21/19 8:28 AM, Maxim Kazantsev wrote:
> Thanks for pointing out! Should be fixed in rL351726.
>
Yep, thanks!
> -----Original Message-----
> From: Mikael Holmén <mikael.holmen at ericsson.com>
> Sent: Monday, January 21, 2019 2:19 PM
> To: Maxim Kazantsev <max.kazantsev at azul.com>; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r351725 - [SCEV][NFC] Introduces expression sizes estimation
>
> Hi,
>
> I get a couple of warnings with this patch:
>
> ../unittests/Analysis/ScalarEvolutionTest.cpp:1426:9: error: unused variable 'R' [-Werror,-Wunused-variable]
> auto *R = cast<Instruction>(Builder.CreateRetVoid());
> ^
>
> and
>
> In file included from ../unittests/Analysis/ScalarEvolutionTest.cpp:26:
> ../utils/unittest/googletest/include/gtest/gtest.h:1392:11: error:
> comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare]
> if (lhs == rhs) {
> ~~~ ^ ~~~
> ../utils/unittest/googletest/include/gtest/gtest.h:1421:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<unsigned int, int>' requested here
> return CmpHelperEQ(lhs_expression, rhs_expression, lhs, rhs);
> ^
> ../unittests/Analysis/ScalarEvolutionTest.cpp:1435:3: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<unsigned int, int>'
> requested here
> EXPECT_EQ(AS->getExpressionSize(), 1);
> ^
> ../utils/unittest/googletest/include/gtest/gtest.h:1924:63: note:
> expanded from macro 'EXPECT_EQ'
> EqHelper<GTEST_IS_NULL_LITERAL_(val1)>::Compare, \
> ^
> ../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:162:23:
> note: expanded from macro 'EXPECT_PRED_FORMAT2'
> GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_NONFATAL_FAILURE_)
> ^
> ../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:147:17:
> note: expanded from macro 'GTEST_PRED_FORMAT2_'
> GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2), \
> ^
> ../utils/unittest/googletest/include/gtest/gtest_pred_impl.h:77:52:
> note: expanded from macro 'GTEST_ASSERT_'
> if (const ::testing::AssertionResult gtest_ar = (expression)) \
> ^
> 2 errors generated.
>
> /Mikael
>
> On 1/21/19 7:19 AM, Max Kazantsev via llvm-commits wrote:
>> Author: mkazantsev
>> Date: Sun Jan 20 22:19:50 2019
>> New Revision: 351725
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=351725&view=rev
>> Log:
>> [SCEV][NFC] Introduces expression sizes estimation
>>
>> This patch introduces the field `ExpressionSize` in SCEV. This field
>> is calculated only once on SCEV creation, and it represents the
>> complexity of this SCEV from arithmetical point of view (not from the
>> point of the number of actual different SCEV nodes that are used in
>> the expression). Roughly saying, it is the number of operands and
>> operations symbols when we print this SCEV.
>>
>> A formal definition is following: if SCEV `X` has operands
>> `Op1`, `Op2`, ..., `OpN`,
>> then
>> Size(X) = 1 + Size(Op1) + Size(Op2) + ... + Size(OpN).
>> Size of SCEVConstant and SCEVUnknown is one.
>>
>> Expression size may be used as a universal way to limit SCEV
>> transformations for huge SCEVs. Currently, we have a bunch of options
>> that represents various limits (such as recursion depth limit) that
>> may not make any sense from the point of view of a LLVM users who is
>> not familiar with SCEV internals, and all these different options
>> pursue one goal. A more general rule that may potentially allow us to
>> get rid of this redundancy in options is "do not make transformations
>> with SCEVs of huge size". It can apply to all SCEV traversals and
>> transformations that may need to visit a SCEV node more than once, hence they are prone to combinatorial explosions.
>>
>> This patch only introduces SCEV sizes calculation as NFC, its
>> utilization will be introduced in follow-up patches.
>>
>> Differential Revision: https://reviews.llvm.org/D35989 Reviewed By:
>> reames
>>
>> Modified:
>> llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
>> llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpressions.h
>> llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>> llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/S
>> calarEvolution.h?rev=351725&r1=351724&r2=351725&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Sun Jan 20
>> +++ 22:19:50 2019
>> @@ -84,6 +84,9 @@ class SCEV : public FoldingSetNode {
>> const unsigned short SCEVType;
>>
>> protected:
>> + // Estimated complexity of this node's expression tree size.
>> + const unsigned short ExpressionSize;
>> +
>> /// This field is initialized to zero and may be used in subclasses to store
>> /// miscellaneous information.
>> unsigned short SubclassData = 0;
>> @@ -115,8 +118,9 @@ public:
>> NoWrapMask = (1 << 3) - 1
>> };
>>
>> - explicit SCEV(const FoldingSetNodeIDRef ID, unsigned SCEVTy)
>> - : FastID(ID), SCEVType(SCEVTy) {}
>> + explicit SCEV(const FoldingSetNodeIDRef ID, unsigned SCEVTy,
>> + unsigned short ExpressionSize)
>> + : FastID(ID), SCEVType(SCEVTy), ExpressionSize(ExpressionSize)
>> + {}
>> SCEV(const SCEV &) = delete;
>> SCEV &operator=(const SCEV &) = delete;
>>
>> @@ -137,6 +141,19 @@ public:
>> /// Return true if the specified scev is negated, but not a constant.
>> bool isNonConstantNegative() const;
>>
>> + // Returns estimated size of the mathematical expression
>> + represented by this // SCEV. The rules of its calculation are following:
>> + // 1) Size of a SCEV without operands (like constants and
>> + SCEVUnknown) is 1; // 2) Size SCEV with operands Op1, Op2, ..., OpN is calculated by formula:
>> + // (1 + Size(Op1) + ... + Size(OpN)).
>> + // This value gives us an estimation of time we need to traverse
>> + through this // SCEV and all its operands recursively. We may use
>> + it to avoid performing // heavy transformations on SCEVs of
>> + excessive size for sake of saving the // compilation time.
>> + unsigned getExpressionSize() const {
>> + return ExpressionSize;
>> + }
>> +
>> /// Print out the internal representation of this scalar to the specified
>> /// stream. This should really only be used for debugging purposes.
>> void print(raw_ostream &OS) const;
>>
>> Modified:
>> llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpressions.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/S
>> calarEvolutionExpressions.h?rev=351725&r1=351724&r2=351725&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpressions.h
>> (original)
>> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolutionExpressions.h Sun
>> +++ Jan 20 22:19:50 2019
>> @@ -50,7 +50,7 @@ class Type;
>> ConstantInt *V;
>>
>> SCEVConstant(const FoldingSetNodeIDRef ID, ConstantInt *v) :
>> - SCEV(ID, scConstant), V(v) {}
>> + SCEV(ID, scConstant, 1), V(v) {}
>>
>> public:
>> ConstantInt *getValue() const { return V; } @@ -64,6 +64,13 @@
>> class Type;
>> }
>> };
>>
>> + static unsigned short computeExpressionSize(ArrayRef<const SCEV *> Args) {
>> + APInt Size(16, 1);
>> + for (auto *Arg : Args)
>> + Size = Size.uadd_sat(APInt(16, Arg->getExpressionSize()));
>> + return (unsigned short)Size.getZExtValue(); }
>> +
>> /// This is the base class for unary cast operator classes.
>> class SCEVCastExpr : public SCEV {
>> protected:
>> @@ -141,9 +148,10 @@ class Type;
>> const SCEV *const *Operands;
>> size_t NumOperands;
>>
>> - SCEVNAryExpr(const FoldingSetNodeIDRef ID,
>> - enum SCEVTypes T, const SCEV *const *O, size_t N)
>> - : SCEV(ID, T), Operands(O), NumOperands(N) {}
>> + SCEVNAryExpr(const FoldingSetNodeIDRef ID, enum SCEVTypes T,
>> + const SCEV *const *O, size_t N)
>> + : SCEV(ID, T, computeExpressionSize(makeArrayRef(O, N))), Operands(O),
>> + NumOperands(N) {}
>>
>> public:
>> size_t getNumOperands() const { return NumOperands; } @@ -257,7
>> +265,8 @@ class Type;
>> const SCEV *RHS;
>>
>> SCEVUDivExpr(const FoldingSetNodeIDRef ID, const SCEV *lhs, const SCEV *rhs)
>> - : SCEV(ID, scUDivExpr), LHS(lhs), RHS(rhs) {}
>> + : SCEV(ID, scUDivExpr, computeExpressionSize({lhs, rhs})), LHS(lhs),
>> + RHS(rhs) {}
>>
>> public:
>> const SCEV *getLHS() const { return LHS; } @@ -410,7 +419,7 @@
>> class Type;
>>
>> SCEVUnknown(const FoldingSetNodeIDRef ID, Value *V,
>> ScalarEvolution *se, SCEVUnknown *next) :
>> - SCEV(ID, scUnknown), CallbackVH(V), SE(se), Next(next) {}
>> + SCEV(ID, scUnknown, 1), CallbackVH(V), SE(se), Next(next) {}
>>
>> // Implement CallbackVH.
>> void deleted() override;
>>
>> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvol
>> ution.cpp?rev=351725&r1=351724&r2=351725&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
>> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Sun Jan 20 22:19:50
>> +++ 2019
>> @@ -392,7 +392,7 @@ bool SCEV::isNonConstantNegative() const
>> }
>>
>> SCEVCouldNotCompute::SCEVCouldNotCompute() :
>> - SCEV(FoldingSetNodeIDRef(), scCouldNotCompute) {}
>> + SCEV(FoldingSetNodeIDRef(), scCouldNotCompute, 0) {}
>>
>> bool SCEVCouldNotCompute::classof(const SCEV *S) {
>> return S->getSCEVType() == scCouldNotCompute; @@ -421,7 +421,7 @@
>> ScalarEvolution::getConstant(Type *Ty, u
>>
>> SCEVCastExpr::SCEVCastExpr(const FoldingSetNodeIDRef ID,
>> unsigned SCEVTy, const SCEV *op, Type
>> *ty)
>> - : SCEV(ID, SCEVTy), Op(op), Ty(ty) {}
>> + : SCEV(ID, SCEVTy, computeExpressionSize(op)), Op(op), Ty(ty) {}
>>
>> SCEVTruncateExpr::SCEVTruncateExpr(const FoldingSetNodeIDRef ID,
>> const SCEV *op, Type *ty)
>>
>> Modified: llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Analysis/Scal
>> arEvolutionTest.cpp?rev=351725&r1=351724&r2=351725&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp (original)
>> +++ llvm/trunk/unittests/Analysis/ScalarEvolutionTest.cpp Sun Jan 20
>> +++ 22:19:50 2019
>> @@ -1389,5 +1389,55 @@ TEST_F(ScalarEvolutionsTest, SCEVCacheNS
>> EXPECT_FALSE(I->hasNoSignedWrap());
>> }
>>
>> +// Check logic of SCEV expression size computation.
>> +TEST_F(ScalarEvolutionsTest, SCEVComputeExpressionSize) {
>> + /*
>> + * Create the following code:
>> + * void func(i64 %a, i64 %b)
>> + * entry:
>> + * %s1 = add i64 %a, 1
>> + * %s2 = udiv i64 %s1, %b
>> + * br label %exit
>> + * exit:
>> + * ret
>> + */
>> +
>> + // Create a module.
>> + Module M("SCEVComputeExpressionSize", Context);
>> +
>> + Type *T_int64 = Type::getInt64Ty(Context);
>> +
>> + FunctionType *FTy =
>> + FunctionType::get(Type::getVoidTy(Context), { T_int64, T_int64
>> + }, false); Function *F =
>> + cast<Function>(M.getOrInsertFunction("func", FTy)); Argument *A =
>> + &*F->arg_begin(); Argument *B = &*std::next(F->arg_begin());
>> + ConstantInt *C = ConstantInt::get(Context, APInt(64, 1));
>> +
>> + BasicBlock *Entry = BasicBlock::Create(Context, "entry", F);
>> + BasicBlock *Exit = BasicBlock::Create(Context, "exit", F);
>> +
>> + IRBuilder<> Builder(Entry);
>> + auto *S1 = cast<Instruction>(Builder.CreateAdd(A, C, "s1")); auto
>> + *S2 = cast<Instruction>(Builder.CreateUDiv(S1, B, "s2"));
>> + Builder.CreateBr(Exit);
>> +
>> + Builder.SetInsertPoint(Exit);
>> + auto *R = cast<Instruction>(Builder.CreateRetVoid());
>> +
>> + ScalarEvolution SE = buildSE(*F);
>> + // Get S2 first to move it to cache.
>> + const SCEV *AS = SE.getSCEV(A);
>> + const SCEV *BS = SE.getSCEV(B);
>> + const SCEV *CS = SE.getSCEV(C);
>> + const SCEV *S1S = SE.getSCEV(S1);
>> + const SCEV *S2S = SE.getSCEV(S2);
>> + EXPECT_EQ(AS->getExpressionSize(), 1);
>> + EXPECT_EQ(BS->getExpressionSize(), 1);
>> + EXPECT_EQ(CS->getExpressionSize(), 1);
>> + EXPECT_EQ(S1S->getExpressionSize(), 3);
>> + EXPECT_EQ(S2S->getExpressionSize(), 5); }
>> +
>> } // end anonymous namespace
>> } // end namespace llvm
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
More information about the llvm-commits
mailing list