[llvm] r281645 - Revert "[ARM] Promote small global constants to constant pools"
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 19 10:27:24 PDT 2016
On Mon, Sep 19, 2016 at 10:14 AM, Renato Golin <renato.golin at linaro.org> wrote:
> Also, please, copy (or inform) the author. I completely missed this revert.
But I've done that! In the llvm-commits thread for the change I've reverted.
>
> Can you elaborate what's wrong? Maybe open a bug, so we can work on it
> and re-apply?
This: "which adds text relocations to ARM binaries."
AFAIK this change has been re-committed with a fix already.
>
> 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