[GlobalMerge] Allow targets to enable merging of extern variables, NFC.

[john.brawn]

Summary

Adjust the GlobalMergeOnExternal option so that the default behaviour is to do whatever the Target thinks is best. Explicitly enabled or disabling the option will override this default.

[ab]

A couple nits, but this sounds reasonable. Obviously, please wait for D10966 to be approved.

lib/CodeGen/GlobalMerge.cpp

133 ↗

(On Diff #29099)

"GlobalMerge" isn't a verb yet ;) How about "[Should]MergeExternalGlobals" or something similar?

610–614 ↗

(On Diff #29099)

How about:

bool MergeExternal = MergeExternalByDefault
if (EnableGlobalMergeOnExternal != cl::BOU_UNSET)
  MergeExternal = (EnableGlobalMergeOnExternal == cl::BOU_TRUE);

(or the even terser ternary version).

llvm/trunk/include/llvm/Transforms/Scalar.h

 
 //===----------------------------------------------------------------------===//
 //
 // GlobalMerge - This pass merges internal (by default) globals into structs
 // to enable reuse of a base pointer by indexed addressing modes.
 // It can also be configured to focus on size optimizations only.
 //
 Pass *createGlobalMergePass(const TargetMachine *TM, unsigned MaximalOffset,
-                            bool OnlyOptimizeForSize = false);
+                            bool OnlyOptimizeForSize = false,
+                            bool MergeExternalByDefault = false);
 
 //===----------------------------------------------------------------------===//
 //
 // LoopUnswitch - This pass is a simple loop unswitching pass.
 //
 Pass *createLoopUnswitchPass(bool OptimizeForSize = false);
 
 //===----------------------------------------------------------------------===//

llvm/trunk/lib/CodeGen/GlobalMerge.cpp

 
 static cl::opt<bool>
 EnableGlobalMergeOnConst("global-merge-on-const", cl::Hidden,
                          cl::desc("Enable global merge pass on constants"),
                          cl::init(false));
 
 // FIXME: this could be a transitional option, and we probably need to remove
 // it if only we are sure this optimization could always benefit all targets.
-static cl::opt<bool>
+static cl::opt<cl::boolOrDefault>
 EnableGlobalMergeOnExternal("global-merge-on-external", cl::Hidden,
-     cl::desc("Enable global merge pass on external linkage"),
-     cl::init(false));
+     cl::desc("Enable global merge pass on external linkage"));
 
 STATISTIC(NumMerged, "Number of globals merged");
 namespace {
   class GlobalMerge : public FunctionPass {
     const TargetMachine *TM;
     // FIXME: Infer the maximum possible offset depending on the actual users
     // (these max offsets are different for the users inside Thumb or ARM
     // functions), see the code that passes in the offset in the ARM backend
     // for more information.
     unsigned MaxOffset;
 
     /// Whether we should try to optimize for size only.
     /// Currently, this applies a dead simple heuristic: only consider globals
     /// used in minsize functions for merging.
     /// FIXME: This could learn about optsize, and be used in the cost model.
     bool OnlyOptimizeForSize;
 
+    /// Whether we should merge global variables that have external linkage.
+    bool MergeExternalGlobals;
+
     bool doMerge(SmallVectorImpl<GlobalVariable*> &Globals,
                  Module &M, bool isConst, unsigned AddrSpace) const;
     /// \brief Merge everything in \p Globals for which the corresponding bit
     /// in \p GlobalSet is set.
     bool doMerge(SmallVectorImpl<GlobalVariable *> &Globals,
                  const BitVector &GlobalSet, Module &M, bool isConst,
                  unsigned AddrSpace) const;
 
class GlobalMerge : public FunctionPass {
 
     /// Keep track of the GlobalVariable that must not be merged away
     SmallPtrSet<const GlobalVariable *, 16> MustKeepGlobalVariables;
 
   public:
     static char ID;             // Pass identification, replacement for typeid.
     explicit GlobalMerge(const TargetMachine *TM = nullptr,
                          unsigned MaximalOffset = 0,
-                         bool OnlyOptimizeForSize = false)
+                         bool OnlyOptimizeForSize = false,
+                         bool MergeExternalGlobals = false)
         : FunctionPass(ID), TM(TM), MaxOffset(MaximalOffset),
-          OnlyOptimizeForSize(OnlyOptimizeForSize) {
+          OnlyOptimizeForSize(OnlyOptimizeForSize),
+          MergeExternalGlobals(MergeExternalGlobals) {
       initializeGlobalMergePass(*PassRegistry::getPassRegistry());
     }
 
     bool doInitialization(Module &M) override;
     bool runOnFunction(Function &F) override;
     bool doFinalization(Module &M) override;
 
     const char *getPassName() const override {
bool GlobalMerge::doInitialization(Module &M) {
 
   // Grab all non-const globals.
   for (Module::global_iterator I = M.global_begin(),
          E = M.global_end(); I != E; ++I) {
     // Merge is safe for "normal" internal or external globals only
     if (I->isDeclaration() || I->isThreadLocal() || I->hasSection())
       continue;
 
-    if (!(EnableGlobalMergeOnExternal && I->hasExternalLinkage()) &&
+    if (!(MergeExternalGlobals && I->hasExternalLinkage()) &&
         !I->hasInternalLinkage())
       continue;
 
     PointerType *PT = dyn_cast<PointerType>(I->getType());
     assert(PT && "Global variable is not a pointer!");
 
     unsigned AddressSpace = PT->getAddressSpace();
 

 }
 
 bool GlobalMerge::doFinalization(Module &M) {
   MustKeepGlobalVariables.clear();
   return false;
 }
 
 Pass *llvm::createGlobalMergePass(const TargetMachine *TM, unsigned Offset,
-                                  bool OnlyOptimizeForSize) {
-  return new GlobalMerge(TM, Offset, OnlyOptimizeForSize);
+                                  bool OnlyOptimizeForSize,
+                                  bool MergeExternalByDefault) {
+  bool MergeExternal = (EnableGlobalMergeOnExternal == cl::BOU_UNSET) ?
+    MergeExternalByDefault : (EnableGlobalMergeOnExternal == cl::BOU_TRUE);
+  return new GlobalMerge(TM, Offset, OnlyOptimizeForSize, MergeExternal);
 }