Remove "localize global" optimization

Summary:
As discussed in http://llvm-reviews.chandlerc.com/D1754,
this optimization isn't really valid for C, and fires too rarely anyway.

Reviewers: rafael, nicholas

Reviewed By: nicholas

CC: rnk, llvm-commits, nicholas
Differential Revision: http://llvm-reviews.chandlerc.com/D1769

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@191834 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Transforms/IPO/GlobalOpt.cpp b/lib/Transforms/IPO/GlobalOpt.cpp
index cf15580b03920c9d628794d46fbabf12ca0daa8d..3129559fb99a0c2698b703f5610364533e15628c 100644 (file)
--- a/lib/Transforms/IPO/GlobalOpt.cpp
+++ b/lib/Transforms/IPO/GlobalOpt.cpp
@@ -50,7 +50,6 @@ STATISTIC(NumSubstitute,"Number of globals with initializers stored into them");
 STATISTIC(NumDeleted   , "Number of globals deleted");
 STATISTIC(NumFnDeleted , "Number of functions deleted");
 STATISTIC(NumGlobUses  , "Number of global uses devirtualized");
-STATISTIC(NumLocalized , "Number of globals localized");
 STATISTIC(NumShrunkToBool  , "Number of global vars shrunk to booleans");
 STATISTIC(NumFastCallFns   , "Number of functions converted to fastcc");
 STATISTIC(NumCtorsEvaluated, "Number of static ctors evaluated");
@@ -137,24 +136,12 @@ struct GlobalStatus {
   /// ever stored to this global, keep track of what value it is.
   Value *StoredOnceValue;
 
-  /// AccessingFunction/HasMultipleAccessingFunctions - These start out
-  /// null/false.  When the first accessing function is noticed, it is recorded.
-  /// When a second different accessing function is noticed,
-  /// HasMultipleAccessingFunctions is set to true.
-  const Function *AccessingFunction;
-  bool HasMultipleAccessingFunctions;
-
-  /// HasNonInstructionUser - Set to true if this global has a user that is not
-  /// an instruction (e.g. a constant expr or GV initializer).
-  bool HasNonInstructionUser;
-
   /// AtomicOrdering - Set to the strongest atomic ordering requirement.
   AtomicOrdering Ordering;
 
-  GlobalStatus() : isCompared(false), isLoaded(false), StoredType(NotStored),
-                   StoredOnceValue(0), AccessingFunction(0),
-                   HasMultipleAccessingFunctions(false),
-                   HasNonInstructionUser(false), Ordering(NotAtomic) {}
+  GlobalStatus()
+      : isCompared(false), isLoaded(false), StoredType(NotStored),
+        StoredOnceValue(0), Ordering(NotAtomic) {}
 };
 
 }
@@ -195,21 +182,12 @@ static bool AnalyzeGlobal(const Value *V, GlobalStatus &GS,
        ++UI) {
     const User *U = *UI;
     if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(U)) {
-      GS.HasNonInstructionUser = true;
-
       // If the result of the constantexpr isn't pointer type, then we won't
       // know to expect it in various places.  Just reject early.
       if (!isa<PointerType>(CE->getType())) return true;
 
       if (AnalyzeGlobal(CE, GS, PHIUsers)) return true;
     } else if (const Instruction *I = dyn_cast<Instruction>(U)) {
-      if (!GS.HasMultipleAccessingFunctions) {
-        const Function *F = I->getParent()->getParent();
-        if (GS.AccessingFunction == 0)
-          GS.AccessingFunction = F;
-        else if (GS.AccessingFunction != F)
-          GS.HasMultipleAccessingFunctions = true;
-      }
       if (const LoadInst *LI = dyn_cast<LoadInst>(I)) {
         GS.isLoaded = true;
         // Don't hack on volatile loads.
@@ -286,12 +264,10 @@ static bool AnalyzeGlobal(const Value *V, GlobalStatus &GS,
         return true;  // Any other non-load instruction might take address!
       }
     } else if (const Constant *C = dyn_cast<Constant>(U)) {
-      GS.HasNonInstructionUser = true;
       // We might have a dead and dangling constant hanging off of here.
       if (!SafeToDestroyConstant(C))
         return true;
     } else {
-      GS.HasNonInstructionUser = true;
       // Otherwise must be some other user.
       return true;
     }
@@ -1938,35 +1914,6 @@ bool GlobalOpt::ProcessGlobal(GlobalVariable *GV,
 bool GlobalOpt::ProcessInternalGlobal(GlobalVariable *GV,
                                       Module::global_iterator &GVI,
                                       const GlobalStatus &GS) {
-  // If this is a first class global and has only one accessing function
-  // and this function is main (which we know is not recursive), we replace
-  // the global with a local alloca in this function.
-  //
-  // NOTE: It doesn't make sense to promote non single-value types since we
-  // are just replacing static memory to stack memory.
-  //
-  // If the global is in different address space, don't bring it to stack.
-  if (!GS.HasMultipleAccessingFunctions &&
-      GS.AccessingFunction && !GS.HasNonInstructionUser &&
-      GV->getType()->getElementType()->isSingleValueType() &&
-      GS.AccessingFunction->getName() == "main" &&
-      GS.AccessingFunction->hasExternalLinkage() &&
-      GV->getType()->getAddressSpace() == 0) {
-    DEBUG(dbgs() << "LOCALIZING GLOBAL: " << *GV);
-    Instruction &FirstI = const_cast<Instruction&>(*GS.AccessingFunction
-                                                   ->getEntryBlock().begin());
-    Type *ElemTy = GV->getType()->getElementType();
-    // FIXME: Pass Global's alignment when globals have alignment
-    AllocaInst *Alloca = new AllocaInst(ElemTy, NULL, GV->getName(), &FirstI);
-    if (!isa<UndefValue>(GV->getInitializer()))
-      new StoreInst(GV->getInitializer(), Alloca, &FirstI);
-
-    GV->replaceAllUsesWith(Alloca);
-    GV->eraseFromParent();
-    ++NumLocalized;
-    return true;
-  }
-
   // If the global is never loaded (but may be stored to), it is dead.
   // Delete it now.
   if (!GS.isLoaded) {

diff --git a/test/Transforms/GlobalOpt/metadata.ll b/test/Transforms/GlobalOpt/metadata.ll
deleted file mode 100644 (file)
index ecf3f94..0000000
--- a/test/Transforms/GlobalOpt/metadata.ll
+++ /dev/null
@@ -1,26 +0,0 @@
-; RUN: opt -S -globalopt < %s | FileCheck %s
-
-; PR6112 - When globalopt does RAUW(@G, %G), the metadata reference should drop
-; to null.  Function local metadata that references @G from a different function
-; to that containing %G should likewise drop to null.
-@G = internal global i8** null
-
-define i32 @main(i32 %argc, i8** %argv) {
-; CHECK-LABEL: @main(
-; CHECK: %G = alloca
-  store i8** %argv, i8*** @G
-  ret i32 0
-}
-
-define void @foo(i32 %x) {
-  call void @llvm.foo(metadata !{i8*** @G, i32 %x})
-; CHECK: call void @llvm.foo(metadata !{null, i32 %x})
-  ret void
-}
-
-declare void @llvm.foo(metadata) nounwind readnone
-
-!named = !{!0}
-
-!0 = metadata !{i8*** @G}
-; CHECK: !0 = metadata !{null}