Revert "[GlobalOpt][FIX] Do not embed initializers into AS!=0 globals"

This reverts commit 7dbba33.

There seems to be a problem with the tests, investigating now:
  https://lab.llvm.org/buildbot/#/builders/61/builds/14574

Author: jdoerfert

llvm/include/llvm/Analysis/TargetTransformInfo.h

@@ -383,10 +383,6 @@ class TargetTransformInfo {
 
   bool isNoopAddrSpaceCast(unsigned FromAS, unsigned ToAS) const;
 
-  /// Return true if globals in this address space can have initializers other
-  /// than `undef`.
-  bool canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const;
-
   unsigned getAssumedAddrSpace(const Value *V) const;
 
   /// Rewrite intrinsic call \p II such that \p OldV will be replaced with \p
@@ -1461,8 +1457,6 @@ class TargetTransformInfo::Concept {
   virtual bool collectFlatAddressOperands(SmallVectorImpl<int> &OpIndexes,
                                           Intrinsic::ID IID) const = 0;
   virtual bool isNoopAddrSpaceCast(unsigned FromAS, unsigned ToAS) const = 0;
-  virtual bool
-  canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const = 0;
   virtual unsigned getAssumedAddrSpace(const Value *V) const = 0;
   virtual Value *rewriteIntrinsicWithAddressSpace(IntrinsicInst *II,
                                                   Value *OldV,
@@ -1788,11 +1782,6 @@ class TargetTransformInfo::Model final : public TargetTransformInfo::Concept {
     return Impl.isNoopAddrSpaceCast(FromAS, ToAS);
   }
 
-  bool
-  canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const override {
-    return Impl.canHaveNonUndefGlobalInitializerInAddressSpace(AS);
-  }
-
   unsigned getAssumedAddrSpace(const Value *V) const override {
     return Impl.getAssumedAddrSpace(V);
   }

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

@@ -105,9 +105,6 @@ class TargetTransformInfoImplBase {
   }
 
   bool isNoopAddrSpaceCast(unsigned, unsigned) const { return false; }
-  bool canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const {
-    return AS == 0;
-  };
 
   unsigned getAssumedAddrSpace(const Value *V) const { return -1; }
 

llvm/include/llvm/Transforms/Utils/GlobalStatus.h

@@ -9,7 +9,6 @@
 #ifndef LLVM_TRANSFORMS_UTILS_GLOBALSTATUS_H
 #define LLVM_TRANSFORMS_UTILS_GLOBALSTATUS_H
 
-#include "llvm/IR/Instructions.h"
 #include "llvm/Support/AtomicOrdering.h"
 
 namespace llvm {
@@ -46,7 +45,7 @@ struct GlobalStatus {
 
     /// This global is stored to, but only its initializer and one other value
     /// is ever stored to it.  If this global isStoredOnce, we track the value
-    /// stored to it via StoredOnceStore below.  This is only tracked for scalar
+    /// stored to it in StoredOnceValue below.  This is only tracked for scalar
     /// globals.
     StoredOnce,
 
@@ -56,16 +55,8 @@ struct GlobalStatus {
   } StoredType = NotStored;
 
   /// If only one value (besides the initializer constant) is ever stored to
-  /// this global, keep track of what value it is via the store instruction.
-  const StoreInst *StoredOnceStore = nullptr;
-
-  /// If only one value (besides the initializer constant) is ever stored to
-  /// this global return the stored value.
-  Value *getStoredOnceValue() const {
-    return (StoredType == StoredOnce && StoredOnceStore)
-               ? StoredOnceStore->getOperand(0)
-               : nullptr;
-  }
+  /// this global, keep track of what value it is.
+  Value *StoredOnceValue = nullptr;
 
   /// These start out null/false.  When the first accessing function is noticed,
   /// it is recorded. When a second different accessing function is noticed,

llvm/lib/Analysis/TargetTransformInfo.cpp

@@ -259,11 +259,6 @@ bool TargetTransformInfo::isNoopAddrSpaceCast(unsigned FromAS,
   return TTIImpl->isNoopAddrSpaceCast(FromAS, ToAS);
 }
 
-bool TargetTransformInfo::canHaveNonUndefGlobalInitializerInAddressSpace(
-    unsigned AS) const {
-  return TTIImpl->canHaveNonUndefGlobalInitializerInAddressSpace(AS);
-}
-
 unsigned TargetTransformInfo::getAssumedAddrSpace(const Value *V) const {
   return TTIImpl->getAssumedAddrSpace(V);
 }

llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h

@@ -182,12 +182,6 @@ class GCNTTIImpl final : public BasicTTIImplBase<GCNTTIImpl> {
 
   bool collectFlatAddressOperands(SmallVectorImpl<int> &OpIndexes,
                                   Intrinsic::ID IID) const;
-
-  bool canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const {
-    return AS != AMDGPUAS::LOCAL_ADDRESS && AS != AMDGPUAS::REGION_ADDRESS &&
-           AS != AMDGPUAS::PRIVATE_ADDRESS;
-  }
-
   Value *rewriteIntrinsicWithAddressSpace(IntrinsicInst *II, Value *OldV,
                                           Value *NewV) const;
 

llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.h

@@ -48,11 +48,6 @@ class NVPTXTTIImpl : public BasicTTIImplBase<NVPTXTTIImpl> {
     return AddressSpace::ADDRESS_SPACE_GENERIC;
   }
 
-  bool canHaveNonUndefGlobalInitializerInAddressSpace(unsigned AS) const {
-    return AS != AddressSpace::ADDRESS_SPACE_SHARED &&
-           AS != AddressSpace::ADDRESS_SPACE_LOCAL && AS != ADDRESS_SPACE_PARAM;
-  }
-
   Optional<Instruction *> instCombineIntrinsic(InstCombiner &IC,
                                                IntrinsicInst &II) const;
 

llvm/lib/Transforms/IPO/GlobalOpt.cpp

@@ -1507,7 +1507,6 @@ static void makeAllConstantUsesInstructions(Constant *C) {
 /// it if possible.  If we make a change, return true.
 static bool
 processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
-                      function_ref<TargetTransformInfo &(Function &)> GetTTI,
                       function_ref<TargetLibraryInfo &(Function &)> GetTLI,
                       function_ref<DominatorTree &(Function &)> LookupDomTree) {
   auto &DL = GV->getParent()->getDataLayout();
@@ -1606,26 +1605,18 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
     if (SRAGlobal(GV, DL))
       return true;
   }
-  Value *StoredOnceValue = GS.getStoredOnceValue();
-  if (GS.StoredType == GlobalStatus::StoredOnce && StoredOnceValue) {
+  if (GS.StoredType == GlobalStatus::StoredOnce && GS.StoredOnceValue) {
     // Avoid speculating constant expressions that might trap (div/rem).
-    auto *SOVConstant = dyn_cast<Constant>(StoredOnceValue);
+    auto *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue);
     if (SOVConstant && SOVConstant->canTrap())
       return Changed;
 
-    Function &StoreFn =
-        const_cast<Function &>(*GS.StoredOnceStore->getFunction());
     // If the initial value for the global was an undef value, and if only
     // one other value was stored into it, we can just change the
     // initializer to be the stored value, then delete all stores to the
     // global.  This allows us to mark it constant.
-    // This is restricted to address spaces that allow globals to have
-    // initializers. NVPTX, for example, does not support initializers for
-    // shared memory (AS 3).
     if (SOVConstant && SOVConstant->getType() == GV->getValueType() &&
-        isa<UndefValue>(GV->getInitializer()) &&
-        GetTTI(StoreFn).canHaveNonUndefGlobalInitializerInAddressSpace(
-            GV->getType()->getAddressSpace())) {
+        isa<UndefValue>(GV->getInitializer())) {
       // Change the initial value here.
       GV->setInitializer(SOVConstant);
 
@@ -1644,7 +1635,8 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
 
     // Try to optimize globals based on the knowledge that only one value
     // (besides its initializer) is ever stored to the global.
-    if (optimizeOnceStoredGlobal(GV, StoredOnceValue, GS.Ordering, DL, GetTLI))
+    if (optimizeOnceStoredGlobal(GV, GS.StoredOnceValue, GS.Ordering, DL,
+                                 GetTLI))
       return true;
 
     // Otherwise, if the global was not a boolean, we can shrink it to be a
@@ -1664,7 +1656,6 @@ processInternalGlobal(GlobalVariable *GV, const GlobalStatus &GS,
 /// make a change, return true.
 static bool
 processGlobal(GlobalValue &GV,
-              function_ref<TargetTransformInfo &(Function &)> GetTTI,
               function_ref<TargetLibraryInfo &(Function &)> GetTLI,
               function_ref<DominatorTree &(Function &)> LookupDomTree) {
   if (GV.getName().startswith("llvm."))
@@ -1697,8 +1688,7 @@ processGlobal(GlobalValue &GV,
   if (GVar->isConstant() || !GVar->hasInitializer())
     return Changed;
 
-  return processInternalGlobal(GVar, GS, GetTTI, GetTLI, LookupDomTree) ||
-         Changed;
+  return processInternalGlobal(GVar, GS, GetTLI, LookupDomTree) || Changed;
 }
 
 /// Walk all of the direct calls of the specified function, changing them to
@@ -2001,7 +1991,7 @@ OptimizeFunctions(Module &M,
       }
     }
 
-    Changed |= processGlobal(*F, GetTTI, GetTLI, LookupDomTree);
+    Changed |= processGlobal(*F, GetTLI, LookupDomTree);
 
     if (!F->hasLocalLinkage())
       continue;
@@ -2070,7 +2060,6 @@ OptimizeFunctions(Module &M,
 
 static bool
 OptimizeGlobalVars(Module &M,
-                   function_ref<TargetTransformInfo &(Function &)> GetTTI,
                    function_ref<TargetLibraryInfo &(Function &)> GetTLI,
                    function_ref<DominatorTree &(Function &)> LookupDomTree,
                    SmallPtrSetImpl<const Comdat *> &NotDiscardableComdats) {
@@ -2099,7 +2088,7 @@ OptimizeGlobalVars(Module &M,
       continue;
     }
 
-    Changed |= processGlobal(*GV, GetTTI, GetTLI, LookupDomTree);
+    Changed |= processGlobal(*GV, GetTLI, LookupDomTree);
   }
   return Changed;
 }
@@ -2677,8 +2666,8 @@ static bool optimizeGlobalsInModule(
     });
 
     // Optimize non-address-taken globals.
-    LocalChange |= OptimizeGlobalVars(M, GetTTI, GetTLI, LookupDomTree,
-                                      NotDiscardableComdats);
+    LocalChange |=
+        OptimizeGlobalVars(M, GetTLI, LookupDomTree, NotDiscardableComdats);
 
     // Resolve aliases, when possible.
     LocalChange |= OptimizeGlobalAliases(M, NotDiscardableComdats);

llvm/lib/Transforms/Utils/GlobalStatus.cpp

@@ -127,9 +127,9 @@ static bool analyzeGlobalAux(const Value *V, GlobalStatus &GS,
                 GS.StoredType = GlobalStatus::InitializerStored;
             } else if (GS.StoredType < GlobalStatus::StoredOnce) {
               GS.StoredType = GlobalStatus::StoredOnce;
-              GS.StoredOnceStore = SI;
+              GS.StoredOnceValue = StoredVal;
             } else if (GS.StoredType == GlobalStatus::StoredOnce &&
-                       GS.getStoredOnceValue() == StoredVal) {
+                       GS.StoredOnceValue == StoredVal) {
               // noop.
             } else {
               GS.StoredType = GlobalStatus::Stored;