[llvm] r281526 - Revert "[ARM] Promote small global constants to constant pools"

Evgeniy Stepanov via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 14 13:02:30 PDT 2016


Author: eugenis
Date: Wed Sep 14 15:02:30 2016
New Revision: 281526

URL: http://llvm.org/viewvc/llvm-project?rev=281526&view=rev
Log:
Revert "[ARM] Promote small global constants to constant pools"

Breaks Android tests by introducing text relocations to ARM binaries.

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

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=281526&r1=281525&r2=281526&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp Wed Sep 14 15:02:30 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=281526&r1=281525&r2=281526&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMAsmPrinter.h Wed Sep 14 15:02:30 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=281526&r1=281525&r2=281526&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Wed Sep 14 15:02:30 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,100 +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();
-  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());
@@ -3080,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=281526&r1=281525&r2=281526&view=diff
==============================================================================
--- llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h (original)
+++ llvm/trunk/lib/Target/ARM/ARMMachineFunctionInfo.h Wed Sep 14 15:02:30 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=281525&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)"}




More information about the llvm-commits mailing list