[llvm] r315680 - Revert r314923: "Recommit : Use the basic cost if a GEP is not used as addressing mode"
Jun Lim via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 13 10:21:04 PDT 2017
r314923 could potentially increase the inline cost because with this change getGEPCost will no longer returns Free for a GEP which is actually a non-free. I doubt simply reverting is the right decision because this change fixed the unexpected behavior in getGEPCost() and caused the regression in the benchmark of which performance reply on the incorrect cost value. If we are doing right thing with r314923, we should look at other way arounds to get the hot function inlined.
From: Daniel Jasper [mailto:djasper at google.com]
Sent: Friday, October 13, 2017 1:00 PM
To: llvm-commits <llvm-commits at lists.llvm.org>; Jun Bum Lim <junbuml at codeaurora.org>; Benjamin Kramer <benny.kra at googlemail.com>
Subject: Re: [llvm] r315680 - Revert r314923: "Recommit : Use the basic cost if a GEP is not used as addressing mode"
We are still investigating, but one of the main differences seems to be that LLVM stops inlining AppendFromSelf:
https://github.com/google/gipfeli/blob/master/stream.h#L90
On Fri, Oct 13, 2017 at 3:04 PM, Daniel Jasper via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org> > wrote:
Author: djasper
Date: Fri Oct 13 07:04:21 2017
New Revision: 315680
URL: http://llvm.org/viewvc/llvm-project?rev=315680 <http://llvm.org/viewvc/llvm-project?rev=315680&view=rev> &view=rev
Log:
Revert r314923: "Recommit : Use the basic cost if a GEP is not used as addressing mode"
Significantly reduces performancei (~30%) of gipfeli
(https://github.com/google/gipfeli)
I have not yet managed to reproduce this regression with the open-source
version of the benchmark on github, but will work with others to get a
reproducer to you later today.
Removed:
llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll
Modified:
llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h
llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h
llvm/trunk/include/llvm/IR/Operator.h
llvm/trunk/lib/Analysis/TargetTransformInfo.cpp
llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp
llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll
llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll
Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h (original)
+++ llvm/trunk/include/llvm/Analysis/TargetTransformInfo.h Fri Oct 13 07:04:21 2017
@@ -193,13 +193,6 @@ public:
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef<const Value *> Operands) const;
- /// \brief Estimate the cost of a GEP operation when lowered.
- ///
- /// This user-based overload adds the ability to check if the GEP can be
- /// folded into its users.
- int getGEPCost(const GEPOperator *GEP,
- ArrayRef<const Value *> Operands) const;
-
/// \brief Estimate the cost of a EXT operation when lowered.
///
/// The contract for this function is the same as \c getOperationCost except
@@ -258,9 +251,9 @@ public:
/// \brief Estimate the cost of a given IR user when lowered.
///
/// This can estimate the cost of either a ConstantExpr or Instruction when
- /// lowered. It has two primary advantages over the \c getOperationCost above,
- /// and one significant disadvantage: it can only be used when the IR
- /// construct has already been formed.
+ /// lowered. It has two primary advantages over the \c getOperationCost and
+ /// \c getGEPCost above, and one significant disadvantage: it can only be
+ /// used when the IR construct has already been formed.
///
/// The advantages are that it can inspect the SSA use graph to reason more
/// accurately about the cost. For example, all-constant-GEPs can often be
@@ -939,8 +932,6 @@ public:
virtual int getOperationCost(unsigned Opcode, Type *Ty, Type *OpTy) = 0;
virtual int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef<const Value *> Operands) = 0;
- virtual int getGEPCost(const GEPOperator *GEP,
- ArrayRef<const Value *> Operands) = 0;
virtual int getExtCost(const Instruction *I, const Value *Src) = 0;
virtual int getCallCost(FunctionType *FTy, int NumArgs) = 0;
virtual int getCallCost(const Function *F, int NumArgs) = 0;
@@ -1122,10 +1113,6 @@ public:
ArrayRef<const Value *> Operands) override {
return Impl.getGEPCost(PointeeType, Ptr, Operands);
}
- int getGEPCost(const GEPOperator *GEP,
- ArrayRef<const Value *> Operands) override {
- return Impl.getGEPCost(GEP, Operands);
- }
int getExtCost(const Instruction *I, const Value *Src) override {
return Impl.getExtCost(I, Src);
}
Modified: llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h (original)
+++ llvm/trunk/include/llvm/Analysis/TargetTransformInfoImpl.h Fri Oct 13 07:04:21 2017
@@ -728,38 +728,6 @@ public:
return TTI::TCC_Basic;
}
- int getGEPCost(const GEPOperator *GEP, ArrayRef<const Value *> Operands) {
- if (!isa<Instruction>(GEP))
- return TTI::TCC_Basic;
-
- Type *PointeeType = GEP->getSourceElementType();
- const Value *Ptr = GEP->getPointerOperand();
-
- if (getGEPCost(PointeeType, Ptr, Operands) == TTI::TCC_Free) {
- // Should check if the GEP is actually used in load / store instructions.
- // For simplicity, we check only direct users of the GEP.
- //
- // FIXME: GEPs could also be folded away as a part of addressing mode in
- // load/store instructions together with other instructions (e.g., other
- // GEPs). Handling all such cases must be expensive to be performed
- // in this function, so we stay conservative for now.
- for (const User *U : GEP->users()) {
- const Operator *UOP = cast<Operator>(U);
- const Value *PointerOperand = nullptr;
- if (auto *LI = dyn_cast<LoadInst>(UOP))
- PointerOperand = LI->getPointerOperand();
- else if (auto *SI = dyn_cast<StoreInst>(UOP))
- PointerOperand = SI->getPointerOperand();
-
- if ((!PointerOperand || PointerOperand != GEP) &&
- !GEP->hasAllZeroIndices())
- return TTI::TCC_Basic;
- }
- return TTI::TCC_Free;
- }
- return TTI::TCC_Basic;
- }
-
using BaseT::getIntrinsicCost;
unsigned getIntrinsicCost(Intrinsic::ID IID, Type *RetTy,
@@ -783,9 +751,11 @@ public:
if (A->isStaticAlloca())
return TTI::TCC_Free;
- if (const GEPOperator *GEP = dyn_cast<GEPOperator>(U))
- return static_cast<T *>(this)->getGEPCost(GEP,
+ if (const GEPOperator *GEP = dyn_cast<GEPOperator>(U)) {
+ return static_cast<T *>(this)->getGEPCost(GEP->getSourceElementType(),
+ GEP->getPointerOperand(),
Operands.drop_front());
+ }
if (auto CS = ImmutableCallSite(U)) {
const Function *F = CS.getCalledFunction();
Modified: llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h (original)
+++ llvm/trunk/include/llvm/CodeGen/BasicTTIImpl.h Fri Oct 13 07:04:21 2017
@@ -189,11 +189,6 @@ public:
return BaseT::getGEPCost(PointeeType, Ptr, Operands);
}
- int getGEPCost(const GEPOperator *GEP,
- ArrayRef<const Value *> Operands) {
- return BaseT::getGEPCost(GEP, Operands);
- }
-
int getExtCost(const Instruction *I, const Value *Src) {
if (getTLI()->isExtFree(I))
return TargetTransformInfo::TCC_Free;
Modified: llvm/trunk/include/llvm/IR/Operator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Operator.h?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Operator.h?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Operator.h (original)
+++ llvm/trunk/include/llvm/IR/Operator.h Fri Oct 13 07:04:21 2017
@@ -456,8 +456,6 @@ public:
if (ConstantInt *C = dyn_cast<ConstantInt>(I))
if (C->isZero())
continue;
- if (isa<ConstantAggregateZero>(I))
- continue;
return false;
}
return true;
Modified: llvm/trunk/lib/Analysis/TargetTransformInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/TargetTransformInfo.cpp?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/TargetTransformInfo.cpp?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/TargetTransformInfo.cpp (original)
+++ llvm/trunk/lib/Analysis/TargetTransformInfo.cpp Fri Oct 13 07:04:21 2017
@@ -88,11 +88,6 @@ int TargetTransformInfo::getGEPCost(Type
return TTIImpl->getGEPCost(PointeeType, Ptr, Operands);
}
-int TargetTransformInfo::getGEPCost(const GEPOperator *GEP,
- ArrayRef<const Value *> Operands) const {
- return TTIImpl->getGEPCost(GEP, Operands);
-}
-
int TargetTransformInfo::getExtCost(const Instruction *I,
const Value *Src) const {
return TTIImpl->getExtCost(I, Src);
Modified: llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/NaryReassociate.cpp Fri Oct 13 07:04:21 2017
@@ -264,7 +264,7 @@ static bool isGEPFoldable(GetElementPtrI
SmallVector<const Value*, 4> Indices;
for (auto I = GEP->idx_begin(); I != GEP->idx_end(); ++I)
Indices.push_back(*I);
- return TTI->getGEPCost(cast<GEPOperator>(GEP),
+ return TTI->getGEPCost(GEP->getSourceElementType(), GEP->getPointerOperand(),
Indices) == TargetTransformInfo::TCC_Free;
}
Modified: llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp Fri Oct 13 07:04:21 2017
@@ -239,7 +239,7 @@ static bool isGEPFoldable(GetElementPtrI
SmallVector<const Value*, 4> Indices;
for (auto I = GEP->idx_begin(); I != GEP->idx_end(); ++I)
Indices.push_back(*I);
- return TTI->getGEPCost(cast<GEPOperator>(GEP),
+ return TTI->getGEPCost(GEP->getSourceElementType(), GEP->getPointerOperand(),
Indices) == TargetTransformInfo::TCC_Free;
}
Modified: llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll (original)
+++ llvm/trunk/test/Analysis/CostModel/AArch64/gep.ll Fri Oct 13 07:04:21 2017
@@ -290,49 +290,3 @@ define i64 @test36(i64* %p) {
%v = load i64, i64* %a
ret i64 %v
}
-
-; CHECK-LABEL: test37
-; CHECK: cost of 1 for instruction: {{.*}} = getelementptr inbounds i8*, i8**
-define i8 @test37(i64 %j, i8** readonly %P) {
-entry:
- %arrayidx0 = getelementptr inbounds i8*, i8** %P, i64 %j
- %l1 = call i8* @func(i8** %arrayidx0)
- ret i8 0
-}
-
-; CHECK-LABEL: test38
-; CHECK: cost of 1 for instruction: {{.*}} = getelementptr inbounds i8*, i8**
-define i8 @test38(i8** readonly %P) {
-entry:
- %arrayidx0 = getelementptr inbounds i8*, i8** %P, i64 10
- %l1 = call i8* @func(i8** %arrayidx0)
- ret i8 0
-}
-
-; CHECK-LABEL:test39
-; CHECK: cost of 0 for instruction: {{.*}} = getelementptr inbounds i8*, i8**
-define i8 @test39(i8** readonly %P) {
-entry:
- %arrayidx0 = getelementptr inbounds i8*, i8** %P, i64 0
- %l1 = call i8* @func(i8** %arrayidx0)
- ret i8 0
-}
-
-; CHECK-LABEL:test40
-; CHECK: cost of 1 for instruction: {{.*}} = getelementptr inbounds i8*, i8**
-define i8** @test40(i8** readonly %P) {
-entry:
- %arrayidx0 = getelementptr inbounds i8*, i8** %P, i64 10
- ret i8** %arrayidx0
-}
-
-; CHECK-LABEL:test41
-; CHECK: cost of 1 for instruction: {{.*}} = getelementptr inbounds i8, i8*
-define i8 @test41(i8* %V, i8** readonly %P) {
-entry:
- %arrayidx0 = getelementptr inbounds i8, i8* %V, i64 10
- store i8* %arrayidx0, i8** %P
- ret i8 0
-}
-
-declare i8* @func(i8**)
Modified: llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll?rev=315680 <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll?rev=315680&r1=315679&r2=315680&view=diff> &r1=315679&r2=315680&view=diff
==============================================================================
--- llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll (original)
+++ llvm/trunk/test/Analysis/CostModel/X86/vector_gep.ll Fri Oct 13 07:04:21 2017
@@ -10,7 +10,7 @@ define <4 x i32> @foov(<4 x %struct.S*>
%vector = shufflevector <4 x i64> %temp, <4 x i64> undef, <4 x i32> zeroinitializer
;CHECK: cost of 0 for instruction: {{.*}} getelementptr inbounds %struct.S
%B = getelementptr inbounds %struct.S, <4 x %struct.S*> %s, <4 x i32> zeroinitializer, <4 x i32> zeroinitializer
-;CHECK: cost of 1 for instruction: {{.*}} getelementptr inbounds [1000 x i32]
+;CHECK: cost of 0 for instruction: {{.*}} getelementptr inbounds [1000 x i32]
%arrayidx = getelementptr inbounds [1000 x i32], <4 x [1000 x i32]*> %B, <4 x i64> zeroinitializer, <4 x i64> %vector
%res = call <4 x i32> @llvm.masked.gather.v4i32.v4p0i32(<4 x i32*> %arrayidx, i32 4, <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i32> undef)
ret <4 x i32> %res
Removed: llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll?rev=315679 <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll?rev=315679&view=auto> &view=auto
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/SpeculativeExecGepCE.ll (removed)
@@ -1,28 +0,0 @@
-; RUN: opt < %s -simplifycfg -phi-node-folding-threshold=0 -S | FileCheck %s
-
-target triple = "x86_64-unknown-linux-gnu"
-
- at d_buf = internal constant [8 x i8] [i8 126, i8 127, i8 128, i8 129, i8 130, i8 131, i8 132, i8 133], align 8
- at a = internal constant { i8*, i64} {i8* getelementptr inbounds ([8 x i8], [8 x i8]* @d_buf, i64 0, i64 0), i64 0}
-
-; CHECK-LABEL: @test
-; CHECK-LABEL: end:
-; CHECK: %x1 = phi i8*
-define i8* @test(i1* %dummy, i8* %a, i8* %b, i8 %v) {
-
-entry:
- %cond1 = load volatile i1, i1* %dummy
- br i1 %cond1, label %if, label %end
-
-if:
- %cond2 = load volatile i1, i1* %dummy
- br i1 %cond2, label %then, label %end
-
-then:
- br label %end
-
-end:
- %x1 = phi i8* [ %a, %entry ], [ %b, %if ], [getelementptr inbounds ([8 x i8], [8 x i8]* @d_buf, i64 0, i64 0) , %then ]
-
- ret i8* %x1
-}
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171013/9b2cdf5a/attachment-0001.html>
More information about the llvm-commits
mailing list