From 682384e698e6cedebd19db8c2551936b27bb6942 Mon Sep 17 00:00:00 2001 From: Alex Shlyapnikov Date: Fri, 29 Sep 2017 22:04:45 +0000 Subject: [PATCH] Revert "Use the basic cost if a GEP is not used as addressing mode" This reverts commit r314517. This commit crashes sanitizer bots, for example: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/4167 Stack snippet: ... /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/Support/Casting.h:255:0 llvm::TargetTransformInfoImplCRTPBase::getGEPCost(llvm::GEPOperator const*, llvm::ArrayRef) /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:742:0 llvm::TargetTransformInfoImplCRTPBase::getUserCost(llvm::User const*, llvm::ArrayRef) /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:782:0 /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/lib/Analysis/TargetTransformInfo.cpp:116:0 /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/ADT/SmallVector.h:116:0 /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/ADT/SmallVector.h:343:0 /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/ADT/SmallVector.h:864:0 /mnt/b/sanitizer-buildbot1/sanitizer-x86_64-linux/build/llvm/include/llvm/Analysis/TargetTransformInfo.h:285:0 ... git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@314560 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/TargetTransformInfo.h | 19 ++------ .../llvm/Analysis/TargetTransformInfoImpl.h | 36 ++------------- include/llvm/CodeGen/BasicTTIImpl.h | 4 -- include/llvm/IR/Operator.h | 2 - lib/Analysis/TargetTransformInfo.cpp | 5 -- lib/Transforms/Scalar/NaryReassociate.cpp | 2 +- .../Scalar/StraightLineStrengthReduce.cpp | 2 +- test/Analysis/CostModel/AArch64/gep.ll | 46 ------------------- test/Analysis/CostModel/X86/vector_gep.ll | 2 +- 9 files changed, 11 insertions(+), 107 deletions(-) diff --git a/include/llvm/Analysis/TargetTransformInfo.h b/include/llvm/Analysis/TargetTransformInfo.h index 10ecf64a39c..afc16e89da6 100644 --- a/include/llvm/Analysis/TargetTransformInfo.h +++ b/include/llvm/Analysis/TargetTransformInfo.h @@ -193,13 +193,6 @@ public: int getGEPCost(Type *PointeeType, const Value *Ptr, ArrayRef 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 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 Operands) = 0; - virtual int getGEPCost(const GEPOperator *GEP, - ArrayRef 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 Operands) override { return Impl.getGEPCost(PointeeType, Ptr, Operands); } - int getGEPCost(const GEPOperator *GEP, - ArrayRef Operands) override { - return Impl.getGEPCost(GEP, Operands); - } int getExtCost(const Instruction *I, const Value *Src) override { return Impl.getExtCost(I, Src); } diff --git a/include/llvm/Analysis/TargetTransformInfoImpl.h b/include/llvm/Analysis/TargetTransformInfoImpl.h index 70651805b19..b3b3e07b4dc 100644 --- a/include/llvm/Analysis/TargetTransformInfoImpl.h +++ b/include/llvm/Analysis/TargetTransformInfoImpl.h @@ -726,35 +726,6 @@ public: return TTI::TCC_Basic; } - int getGEPCost(const GEPOperator *GEP, ArrayRef Operands) { - 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(U); - const Value *PointerOperand = nullptr; - if (auto *LI = dyn_cast(UOP)) - PointerOperand = LI->getPointerOperand(); - else if (auto *SI = dyn_cast(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, @@ -778,8 +749,11 @@ public: if (A->isStaticAlloca()) return TTI::TCC_Free; - if (const GEPOperator *GEP = dyn_cast(U)) - return static_cast(this)->getGEPCost(GEP, Operands.drop_front()); + if (const GEPOperator *GEP = dyn_cast(U)) { + return static_cast(this)->getGEPCost(GEP->getSourceElementType(), + GEP->getPointerOperand(), + Operands.drop_front()); + } if (auto CS = ImmutableCallSite(U)) { const Function *F = CS.getCalledFunction(); diff --git a/include/llvm/CodeGen/BasicTTIImpl.h b/include/llvm/CodeGen/BasicTTIImpl.h index e2e46acd0a3..0334ed9eacb 100644 --- a/include/llvm/CodeGen/BasicTTIImpl.h +++ b/include/llvm/CodeGen/BasicTTIImpl.h @@ -189,10 +189,6 @@ public: return BaseT::getGEPCost(PointeeType, Ptr, Operands); } - int getGEPCost(const GEPOperator *GEP, ArrayRef Operands) { - return BaseT::getGEPCost(GEP, Operands); - } - int getExtCost(const Instruction *I, const Value *Src) { if (getTLI()->isExtFree(I)) return TargetTransformInfo::TCC_Free; diff --git a/include/llvm/IR/Operator.h b/include/llvm/IR/Operator.h index 84ab4f36444..54e1165a111 100644 --- a/include/llvm/IR/Operator.h +++ b/include/llvm/IR/Operator.h @@ -456,8 +456,6 @@ public: if (ConstantInt *C = dyn_cast(I)) if (C->isZero()) continue; - if (isa(I)) - continue; return false; } return true; diff --git a/lib/Analysis/TargetTransformInfo.cpp b/lib/Analysis/TargetTransformInfo.cpp index 154021a51b9..fad918dabb5 100644 --- a/lib/Analysis/TargetTransformInfo.cpp +++ b/lib/Analysis/TargetTransformInfo.cpp @@ -88,11 +88,6 @@ int TargetTransformInfo::getGEPCost(Type *PointeeType, const Value *Ptr, return TTIImpl->getGEPCost(PointeeType, Ptr, Operands); } -int TargetTransformInfo::getGEPCost(const GEPOperator *GEP, - ArrayRef Operands) const { - return TTIImpl->getGEPCost(GEP, Operands); -} - int TargetTransformInfo::getExtCost(const Instruction *I, const Value *Src) const { return TTIImpl->getExtCost(I, Src); diff --git a/lib/Transforms/Scalar/NaryReassociate.cpp b/lib/Transforms/Scalar/NaryReassociate.cpp index 3d5a513e3d3..d0bfe360389 100644 --- a/lib/Transforms/Scalar/NaryReassociate.cpp +++ b/lib/Transforms/Scalar/NaryReassociate.cpp @@ -264,7 +264,7 @@ static bool isGEPFoldable(GetElementPtrInst *GEP, SmallVector Indices; for (auto I = GEP->idx_begin(); I != GEP->idx_end(); ++I) Indices.push_back(*I); - return TTI->getGEPCost(cast(GEP), + return TTI->getGEPCost(GEP->getSourceElementType(), GEP->getPointerOperand(), Indices) == TargetTransformInfo::TCC_Free; } diff --git a/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp b/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp index 3f93eaecc5d..8b8d6590aa6 100644 --- a/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp +++ b/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp @@ -239,7 +239,7 @@ static bool isGEPFoldable(GetElementPtrInst *GEP, SmallVector Indices; for (auto I = GEP->idx_begin(); I != GEP->idx_end(); ++I) Indices.push_back(*I); - return TTI->getGEPCost(cast(GEP), + return TTI->getGEPCost(GEP->getSourceElementType(), GEP->getPointerOperand(), Indices) == TargetTransformInfo::TCC_Free; } diff --git a/test/Analysis/CostModel/AArch64/gep.ll b/test/Analysis/CostModel/AArch64/gep.ll index 594be51f91c..08bfc3d2123 100644 --- a/test/Analysis/CostModel/AArch64/gep.ll +++ b/test/Analysis/CostModel/AArch64/gep.ll @@ -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**) diff --git a/test/Analysis/CostModel/X86/vector_gep.ll b/test/Analysis/CostModel/X86/vector_gep.ll index 5c00512ba34..17f70dfc7a7 100644 --- a/test/Analysis/CostModel/X86/vector_gep.ll +++ b/test/Analysis/CostModel/X86/vector_gep.ll @@ -10,7 +10,7 @@ define <4 x i32> @foov(<4 x %struct.S*> %s, i64 %base){ %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> , <4 x i32> undef) ret <4 x i32> %res -- 2.40.0