[llvm] r281484 - [ARM] Promote small global constants to constant pools

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 13:01:59 PDT 2016


Hi,

Certainly not intentional- would need some investigation to see if it's a reasonable thing to do; as you have a bit warning about it it probably isn't!

This patch broke self host on thumb too so I have some remedial action to do as soon as I get to a computer in the morning - I'll fix or revert for this problem too.

Feel free to revert in the meantime.

Cheers,

James

> On 14 Sep2016, at 20:57, Evgenii Stepanov <eugeni.stepanov at gmail.com> wrote:
>
> Hi,
>
> this is adding text relocations to ARM binaries, which makes Android
> loader unhappy:
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/25362/steps/run%20asan%20lit%20tests%20%5Barm%2Fbullhead-userdebug%2FMTC20F%5D/logs/stdio
>
> WARNING: linker: /system/bin/llvm-symbolizer has text relocations.
> This is wasting memory and prevents security hardening. Please fix.
>
> It that intentional?
>
>
> On Wed, Sep 14, 2016 at 7:47 AM, James Molloy via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: jamesm
>> Date: Wed Sep 14 09:47:27 2016
>> New Revision: 281484
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281484&view=rev
>> Log:
>> [ARM] Promote small global constants to constant pools
>>
>> If a constant is unamed_addr and is only used within one function, we can save
>> on the code size and runtime cost of an indirection by changing the global's storage
>> to inside the constant pool. For example, instead of:
>>
>>      ldr r0, .CPI0
>>      bl printf
>>      bx lr
>>    .CPI0: &format_string
>>    format_string: .asciz "hello, world!\n"
>>
>> We can emit:
>>
>>      adr r0, .CPI0
>>      bl printf
>>      bx lr
>>    .CPI0: .asciz "hello, world!\n"
>>
>> This can cause significant code size savings when many small strings are used in one
>> function (4 bytes per string).
>>
>> Added:
>>    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=281484&r1=281483&r2=281484&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Wed Sep 14 09:47:27 2016
>> @@ -97,6 +97,13 @@ 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.
>> ///
>> @@ -109,6 +116,12 @@ 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=281484&r1=281483&r2=281484&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h Wed Sep 14 09:47:27 2016
>> @@ -56,6 +56,12 @@ 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);
>> @@ -90,7 +96,8 @@ 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=281484&r1=281483&r2=281484&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Wed Sep 14 09:47:27 2016
>> @@ -59,12 +59,24 @@ 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:
>> @@ -2963,6 +2975,100 @@ 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();
>> +  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);
>> +
>> +      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());
>> @@ -2974,6 +3080,11 @@ 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=281484&r1=281483&r2=281484&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h (original)
>> +++ llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h Wed Sep 14 09:47:27 2016
>> @@ -121,6 +121,9 @@ 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),
>> @@ -226,6 +229,16 @@ 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
>>
>>
>> Added: llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll?rev=281484&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll (added)
>> +++ llvm/trunk/test/CodeGen/ARM/constantpool-promote.ll Wed Sep 14 09:47:27 2016
>> @@ -0,0 +1,109 @@
>> +; 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
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.



More information about the llvm-commits mailing list