[llvm] r281645 - Revert "[ARM] Promote small global constants to constant pools"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 19 09:46:59 PDT 2016
When reverting a change, please explain why. Did this cause a bug? If
so, which one?
On 09/15/2016 12:13 PM, Evgeniy Stepanov via llvm-commits wrote:
> Author: eugenis
> Date: Thu Sep 15 14:13:32 2016
> New Revision: 281645
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281645&view=rev
> Log:
> Revert "[ARM] Promote small global constants to constant pools"
>
> This reverts r281604, which adds text relocations to ARM binaries.
>
> Removed:
> llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll
> Modified:
> llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
> llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h
> llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h
>
> Modified: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp?rev=281645&r1=281644&r2=281645&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Thu Sep 15 14:13:32 2016
> @@ -97,13 +97,6 @@ void ARMAsmPrinter::EmitXXStructor(const
> OutStreamer->EmitValue(E, Size);
> }
>
> -void ARMAsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
> - if (PromotedGlobals.count(GV))
> - // The global was promoted into a constant pool. It should not be emitted.
> - return;
> - AsmPrinter::EmitGlobalVariable(GV);
> -}
> -
> /// runOnMachineFunction - This uses the EmitInstruction()
> /// method to print assembly for each instruction.
> ///
> @@ -116,12 +109,6 @@ bool ARMAsmPrinter::runOnMachineFunction
> const Function* F = MF.getFunction();
> const TargetMachine& TM = MF.getTarget();
>
> - // Collect all globals that had their storage promoted to a constant pool.
> - // Functions are emitted before variables, so this accumulates promoted
> - // globals from all functions in PromotedGlobals.
> - for (auto *GV : AFI->getGlobalsPromotedToConstantPool())
> - PromotedGlobals.insert(GV);
> -
> // Calculate this function's optimization goal.
> unsigned OptimizationGoal;
> if (F->hasFnAttribute(Attribute::OptimizeNone))
>
> Modified: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h?rev=281645&r1=281644&r2=281645&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h (original)
> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h Thu Sep 15 14:13:32 2016
> @@ -56,12 +56,6 @@ class LLVM_LIBRARY_VISIBILITY ARMAsmPrin
> /// -1 if uninitialized, 0 if conflicting goals
> int OptimizationGoals;
>
> - /// List of globals that have had their storage promoted to a constant
> - /// pool. This lives between calls to runOnMachineFunction and collects
> - /// data from every MachineFunction. It is used during doFinalization
> - /// when all non-function globals are emitted.
> - SmallPtrSet<const GlobalVariable*,2> PromotedGlobals;
> -
> public:
> explicit ARMAsmPrinter(TargetMachine &TM,
> std::unique_ptr<MCStreamer> Streamer);
> @@ -96,8 +90,7 @@ public:
> void EmitStartOfAsmFile(Module &M) override;
> void EmitEndOfAsmFile(Module &M) override;
> void EmitXXStructor(const DataLayout &DL, const Constant *CV) override;
> - void EmitGlobalVariable(const GlobalVariable *GV) override;
> -
> +
> // lowerOperand - Convert a MachineOperand into the equivalent MCOperand.
> bool lowerOperand(const MachineOperand &MO, MCOperand &MCOp);
>
>
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=281645&r1=281644&r2=281645&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Sep 15 14:13:32 2016
> @@ -59,24 +59,12 @@ using namespace llvm;
> STATISTIC(NumTailCalls, "Number of tail calls");
> STATISTIC(NumMovwMovt, "Number of GAs materialized with movw + movt");
> STATISTIC(NumLoopByVals, "Number of loops generated for byval arguments");
> -STATISTIC(NumConstpoolPromoted,
> - "Number of constants with their storage promoted into constant pools");
>
> static cl::opt<bool>
> ARMInterworking("arm-interworking", cl::Hidden,
> cl::desc("Enable / disable ARM interworking (for debugging only)"),
> cl::init(true));
>
> -static cl::opt<bool> EnableConstpoolPromotion(
> - "arm-promote-constant", cl::Hidden,
> - cl::desc("Enable / disable promotion of unnamed_addr constants into "
> - "constant pools"),
> - cl::init(true));
> -static cl::opt<unsigned> ConstpoolPromotionMaxSize(
> - "arm-promote-constant-max-size", cl::Hidden,
> - cl::desc("Maximum size of constant to promote into a constant pool"),
> - cl::init(64));
> -
> namespace {
> class ARMCCState : public CCState {
> public:
> @@ -2975,110 +2963,6 @@ ARMTargetLowering::LowerGlobalTLSAddress
> llvm_unreachable("bogus TLS model");
> }
>
> -/// Return true if all users of V are within function F, looking through
> -/// ConstantExprs.
> -static bool allUsersAreInFunction(const Value *V, const Function *F) {
> - SmallVector<const User*,4> Worklist;
> - for (auto *U : V->users())
> - Worklist.push_back(U);
> - while (!Worklist.empty()) {
> - auto *U = Worklist.pop_back_val();
> - if (isa<ConstantExpr>(U)) {
> - for (auto *UU : U->users())
> - Worklist.push_back(UU);
> - continue;
> - }
> -
> - auto *I = dyn_cast<Instruction>(U);
> - if (!I || I->getParent()->getParent() != F)
> - return false;
> - }
> - return true;
> -}
> -
> -/// Return true if all users of V are within some (any) function, looking through
> -/// ConstantExprs. In other words, are there any global constant users?
> -static bool allUsersAreInFunctions(const Value *V) {
> - SmallVector<const User*,4> Worklist;
> - for (auto *U : V->users())
> - Worklist.push_back(U);
> - while (!Worklist.empty()) {
> - auto *U = Worklist.pop_back_val();
> - if (isa<ConstantExpr>(U)) {
> - for (auto *UU : U->users())
> - Worklist.push_back(UU);
> - continue;
> - }
> -
> - if (!isa<Instruction>(U))
> - return false;
> - }
> - return true;
> -}
> -
> -static SDValue promoteToConstantPool(const GlobalValue *GV, SelectionDAG &DAG,
> - EVT PtrVT, SDLoc dl) {
> - // If we're creating a pool entry for a constant global with unnamed address,
> - // and the global is small enough, we can emit it inline into the constant pool
> - // to save ourselves an indirection.
> - //
> - // This is a win if the constant is only used in one function (so it doesn't
> - // need to be duplicated) or duplicating the constant wouldn't increase code
> - // size (implying the constant is no larger than 4 bytes).
> - const Function *F = DAG.getMachineFunction().getFunction();
> -
> - // We rely on this decision to inline being idemopotent and unrelated to the
> - // use-site. We know that if we inline a variable at one use site, we'll
> - // inline it elsewhere too (and reuse the constant pool entry). Fast-isel
> - // doesn't know about this optimization, so bail out if it's enabled else
> - // we could decide to inline here (and thus never emit the GV) but require
> - // the GV from fast-isel generated code.
> - if (DAG.getMachineFunction().getTarget().Options.EnableFastISel)
> - return SDValue();
> -
> - auto *GVar = dyn_cast<GlobalVariable>(GV);
> - if (EnableConstpoolPromotion && GVar && GVar->hasInitializer() &&
> - GVar->isConstant() && GVar->hasGlobalUnnamedAddr() &&
> - GVar->hasLocalLinkage()) {
> - // The constant islands pass can only really deal with alignment requests
> - // <= 4 bytes and cannot pad constants itself. Therefore we cannot promote
> - // any type wanting greater alignment requirements than 4 bytes. We also
> - // can only promote constants that are multiples of 4 bytes in size or
> - // are paddable to a multiple of 4. Currently we only try and pad constants
> - // that are strings for simplicity.
> - auto *Init = GVar->getInitializer();
> - auto *CDAInit = dyn_cast<ConstantDataArray>(Init);
> - unsigned Size = DAG.getDataLayout().getTypeAllocSize(Init->getType());
> - unsigned Align = DAG.getDataLayout().getABITypeAlignment(Init->getType());
> - unsigned RequiredPadding = 4 - (Size % 4);
> - bool PaddingPossible =
> - RequiredPadding == 4 || (CDAInit && CDAInit->isString());
> -
> - if (PaddingPossible && Align <= 4 && Size <= ConstpoolPromotionMaxSize &&
> - (allUsersAreInFunction(GVar, F) ||
> - (Size <= 4 && allUsersAreInFunctions(GVar)))) {
> - if (RequiredPadding != 4) {
> - StringRef S = CDAInit->getAsString();
> -
> - SmallVector<uint8_t,16> V(S.size());
> - std::copy(S.bytes_begin(), S.bytes_end(), V.begin());
> - while (RequiredPadding--)
> - V.push_back(0);
> - Init = ConstantDataArray::get(*DAG.getContext(), V);
> - }
> -
> - SDValue CPAddr =
> - DAG.getTargetConstantPool(Init, PtrVT, /*Align=*/4);
> - MachineFunction &MF = DAG.getMachineFunction();
> - ARMFunctionInfo *AFI = MF.getInfo<ARMFunctionInfo>();
> - AFI->markGlobalAsPromotedToConstantPool(GVar);
> - ++NumConstpoolPromoted;
> - return DAG.getNode(ARMISD::Wrapper, dl, MVT::i32, CPAddr);
> - }
> - }
> - return SDValue();
> -}
> -
> SDValue ARMTargetLowering::LowerGlobalAddressELF(SDValue Op,
> SelectionDAG &DAG) const {
> EVT PtrVT = getPointerTy(DAG.getDataLayout());
> @@ -3090,11 +2974,6 @@ SDValue ARMTargetLowering::LowerGlobalAd
> bool IsRO =
> (isa<GlobalVariable>(GV) && cast<GlobalVariable>(GV)->isConstant()) ||
> isa<Function>(GV);
> -
> - if (TM.shouldAssumeDSOLocal(*GV->getParent(), GV))
> - if (SDValue V = promoteToConstantPool(GV, DAG, PtrVT, dl))
> - return V;
> -
> if (isPositionIndependent()) {
> bool UseGOT_PREL = !TM.shouldAssumeDSOLocal(*GV->getParent(), GV);
>
>
> Modified: llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h?rev=281645&r1=281644&r2=281645&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h (original)
> +++ llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h Thu Sep 15 14:13:32 2016
> @@ -121,9 +121,6 @@ class ARMFunctionInfo : public MachineFu
> /// copies.
> bool IsSplitCSR;
>
> - /// Globals that have had their storage promoted into the constant pool.
> - SmallVector<const GlobalVariable*,2> PromotedGlobals;
> -
> public:
> ARMFunctionInfo() :
> isThumb(false),
> @@ -229,16 +226,6 @@ public:
> }
> return It;
> }
> -
> - /// Indicate to the backend that \c GV has had its storage changed to inside
> - /// a constant pool. This means it no longer needs to be emitted as a
> - /// global variable.
> - void markGlobalAsPromotedToConstantPool(const GlobalVariable *GV) {
> - PromotedGlobals.push_back(GV);
> - }
> - ArrayRef<const GlobalVariable*> getGlobalsPromotedToConstantPool() {
> - return PromotedGlobals;
> - }
> };
> } // End llvm namespace
>
>
> Removed: llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll?rev=281644&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll (original)
> +++ llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll (removed)
> @@ -1,109 +0,0 @@
> -; RUN: llc -relocation-model=static < %s | FileCheck %s
> -; RUN: llc -relocation-model=pic < %s | FileCheck %s
> -; RUN: llc -relocation-model=ropi < %s | FileCheck %s
> -; RUN: llc -relocation-model=rwpi < %s | FileCheck %s
> -
> -target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-n32-S64"
> -target triple = "armv7--linux-gnueabihf"
> -
> - at .str = private unnamed_addr constant [2 x i8] c"s\00", align 1
> - at .str1 = private unnamed_addr constant [69 x i8] c"this string is far too long to fit in a literal pool by far and away\00", align 1
> - at .str2 = private unnamed_addr constant [27 x i8] c"this string is just right!\00", align 1
> - at .str3 = private unnamed_addr constant [26 x i8] c"this string is used twice\00", align 1
> - at .str4 = private unnamed_addr constant [29 x i8] c"same string in two functions\00", align 1
> - at .arr1 = private unnamed_addr constant [2 x i16] [i16 3, i16 4], align 2
> - at .arr2 = private unnamed_addr constant [2 x i16] [i16 7, i16 8], align 2
> - at .ptr = private unnamed_addr constant [2 x i16*] [i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr2, i32 0, i32 0), i16* null], align 2
> -
> -; CHECK-LABEL: @test1
> -; CHECK: adr r0, [[x:.*]]
> -; CHECK: [[x]]:
> -; CHECK: .asciz "s\000\000"
> -define void @test1() #0 {
> - tail call void @a(i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -declare void @a(i8*) #1
> -
> -; CHECK-LABEL: @test2
> -; CHECK-NOT: .asci
> -; CHECK: .fnend
> -define void @test2() #0 {
> - tail call void @a(i8* getelementptr inbounds ([69 x i8], [69 x i8]* @.str1, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -; CHECK-LABEL: @test3
> -; CHECK: adr r0, [[x:.*]]
> -; CHECK: [[x]]:
> -; CHECK: .asciz "this string is just right!\000"
> -define void @test3() #0 {
> - tail call void @a(i8* getelementptr inbounds ([27 x i8], [27 x i8]* @.str2, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -
> -; CHECK-LABEL: @test4
> -; CHECK: adr r{{.*}}, [[x:.*]]
> -; CHECK: [[x]]:
> -; CHECK: .asciz "this string is used twice\000\000"
> -define void @test4() #0 {
> - tail call void @a(i8* getelementptr inbounds ([26 x i8], [26 x i8]* @.str3, i32 0, i32 0)) #2
> - tail call void @a(i8* getelementptr inbounds ([26 x i8], [26 x i8]* @.str3, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -; CHECK-LABEL: @test5a
> -; CHECK-NOT: adr
> -define void @test5a() #0 {
> - tail call void @a(i8* getelementptr inbounds ([29 x i8], [29 x i8]* @.str4, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -define void @test5b() #0 {
> - tail call void @b(i8* getelementptr inbounds ([29 x i8], [29 x i8]* @.str4, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -; CHECK-LABEL: @test6a
> -; CHECK: adr r0, [[x:.*]]
> -; CHECK: [[x]]:
> -; CHECK: .short 3
> -; CHECK: .short 4
> -define void @test6a() #0 {
> - tail call void @c(i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr1, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -; CHECK-LABEL: @test6b
> -; CHECK: adr r0, [[x:.*]]
> -; CHECK: [[x]]:
> -; CHECK: .short 3
> -; CHECK: .short 4
> -define void @test6b() #0 {
> - tail call void @c(i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr1, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -; This shouldn't be promoted, as the string is used by another global.
> -; CHECK-LABEL: @test7
> -; CHECK-NOT: adr
> -define void @test7() #0 {
> - tail call void @c(i16* getelementptr inbounds ([2 x i16], [2 x i16]* @.arr2, i32 0, i32 0)) #2
> - ret void
> -}
> -
> -declare void @b(i8*) #1
> -declare void @c(i16*) #1
> -
> -attributes #0 = { nounwind "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
> -attributes #1 = { "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
> -attributes #2 = { nounwind }
> -
> -!llvm.module.flags = !{!0, !1}
> -!llvm.ident = !{!2}
> -
> -!0 = !{i32 1, !"wchar_size", i32 4}
> -!1 = !{i32 1, !"min_enum_size", i32 4}
> -!2 = !{!"Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)"}
>
>
> _______________________________________________
> 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