[GlobalOpt] Allow constant globals to be SRA'd

[jmolloy]

Summary

The current logic assumes that any constant global will never be SRA'd. I presume this is because normally constant globals can be pushed into their uses and deleted. However, that sometimes can't happen (which is where you really want SRA, so the elements that can be eliminated, are!).

There seems to be no reason why we can't SRA constants too, so let's do it.

[mehdi_amini]

test/Transforms/GlobalOpt/globalsra.ll

31

It is strange that we can store to a constant?

[llvm-commits]

• msg-28212-280.txt162 BDownload

[jmolloy]

test/Transforms/GlobalOpt/globalsra.ll

31

What Danny said - also, these tests are carbon copies of the existing ones above, they just operate on a constant instead of a global.

[mehdi_amini]

LGTM, thanks.

[mehdi_amini]

Is it to be landed?

[mehdi_amini]

James: ping

[mehdi_amini]

James: ping ^2

[jmolloy]

Hi Mehdi,

Sorry for the delay on this. Committed in r267393.

James

lib/Transforms/IPO/GlobalOpt.cpp

 /// program in a more fine-grained way.  We have determined that this
 /// transformation is safe already.  We return the first global variable we
 /// insert so that the caller can reprocess it.
 static GlobalVariable *SRAGlobal(GlobalVariable *GV, const DataLayout &DL) {
   // Make sure this global only has simple uses that we can SRA.
   if (!GlobalUsersSafeToSRA(GV))
     return nullptr;
 
-  assert(GV->hasLocalLinkage() && !GV->isConstant());
+  assert(GV->hasLocalLinkage());
   Constant *Init = GV->getInitializer();
   Type *Ty = Init->getType();
 
   std::vector<GlobalVariable*> NewGlobals;
   Module::GlobalListType &Globals = GV->getParent()->getGlobalList();
 
   // Get the alignment of the global, either explicit or target-specific.
   unsigned StartAlignment = GV->getAlignment();
if (!GS.IsLoaded) {
     // If the global is dead now, delete it.
     if (GV->use_empty()) {
       GV->eraseFromParent();
       ++NumDeleted;
       Changed = true;
     }
     return Changed;
 
-  } else if (GS.StoredType <= GlobalStatus::InitializerStored) {
+  }
+  if (GS.StoredType <= GlobalStatus::InitializerStored) {
     DEBUG(dbgs() << "MARKING CONSTANT: " << *GV << "\n");
     GV->setConstant(true);
 
     // Clean up any obviously simplifiable users now.
     CleanupConstantGlobalUsers(GV, GV->getInitializer(), DL, TLI);
 
     // If the global is dead now, just nuke it.
     if (GV->use_empty()) {
       DEBUG(dbgs() << "   *** Marking constant allowed us to simplify "
             << "all users and delete global!\n");
       GV->eraseFromParent();
       ++NumDeleted;
+      return true;
     }
 
+    // Fall through to the next check; see if we can optimize further.
     ++NumMarked;
-    return true;
-  } else if (!GV->getInitializer()->getType()->isSingleValueType()) {
+  }
+  if (!GV->getInitializer()->getType()->isSingleValueType()) {
     const DataLayout &DL = GV->getParent()->getDataLayout();
     if (GlobalVariable *FirstNewGV = SRAGlobal(GV, DL)) {
       GVI = FirstNewGV->getIterator(); // Don't skip the newly produced globals!
       return true;
     }
-  } else if (GS.StoredType == GlobalStatus::StoredOnce && GS.StoredOnceValue) {
+  }
+  if (GS.StoredType == GlobalStatus::StoredOnce && GS.StoredOnceValue) {
     // 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.
     if (Constant *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue))
       if (isa<UndefValue>(GV->getInitializer())) {
         // Change the initial value here.
         GV->setInitializer(SOVConstant);

test/Transforms/GlobalOpt/globalsra.ll

define float @storeinit() {
         ret float %X
 }
 
 define double @constantize() {
         %X = load double, double* getelementptr ({ i32, float, { double } }, { i32, float, { double } }* @G, i32 0, i32 2, i32 0)           ; <double> [#uses=1]
         ret double %X
 }
 
+@G2 = internal constant { i32, float, { double } } {
+    i32 1, 
+    float 1.000000e+00, 
+    { double } { double 1.727000e+01 } }                ; <{ i32, float, { double } }*> [#uses=3]
+
+define void @onlystore2() {
+        store i32 123, i32* getelementptr ({ i32, float, { double } }, { i32, float, { double } }* @G2, i32 0, i32 0)
+        ret void
+}
+
+define float @storeinit2() {
+        store float 1.000000e+00, float* getelementptr ({ i32, float, { double } }, { i32, float, { double } }* @G2, i32 0, i32 1)
+        %X = load float, float* getelementptr ({ i32, float, { double } }, { i32, float, { double } }* @G2, i32 0, i32 1)           ; <float> [#uses=1]
+        ret float %X
+}
+
+define double @constantize2() {
+        %X = load double, double* getelementptr ({ i32, float, { double } }, { i32, float, { double } }* @G2, i32 0, i32 2, i32 0)           ; <double> [#uses=1]
+        ret double %X
+}
+