[llvm] r351725 - [SCEV][NFC] Introduces expression sizes estimation
Maxim Kazantsev via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 20 23:28:30 PST 2019
Thanks for pointing out! Should be fixed in rL351726.
-----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