[llvm] r281645 - Revert "[ARM] Promote small global constants to constant pools"
Renato Golin via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 19 10:14:07 PDT 2016
Also, please, copy (or inform) the author. I completely missed this revert.
Can you elaborate what's wrong? Maybe open a bug, so we can work on it
and re-apply?
cheers,
-renato
On 19 September 2016 at 17:46, Philip Reames via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> 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
>
>
>
> _______________________________________________
> 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