[llvm] r250035 - GlobalOpt does not treat externally_initialized globals correctly

[Oliver Stannard via llvm-commits]

Author: olista01
Date: Mon Oct 12 08:20:52 2015
New Revision: 250035

URL: http://llvm.org/viewvc/llvm-project?rev=250035&view=rev
Log:
GlobalOpt does not treat externally_initialized globals correctly

GlobalOpt currently merges stores into the initialisers of internal,
externally_initialized globals, but should not do so as the value of the global
may change between the initialiser and any code in the module being run.


Added:
    llvm/trunk/test/Transforms/GlobalOpt/externally-initialized.ll
Modified:
    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
    llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp

Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=250035&r1=250034&r2=250035&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Mon Oct 12 08:20:52 2015
@@ -1804,7 +1804,7 @@ bool GlobalOpt::ProcessInternalGlobal(Gl
       GVI = FirstNewGV; // Don't skip the newly produced globals!
       return true;
     }
-  } else if (GS.StoredType == GlobalStatus::StoredOnce) {
+  } else 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

Modified: llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp?rev=250035&r1=250034&r2=250035&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp Mon Oct 12 08:20:52 2015
@@ -49,6 +49,10 @@ bool llvm::isSafeToDestroyConstant(const
 
 static bool analyzeGlobalAux(const Value *V, GlobalStatus &GS,
                              SmallPtrSetImpl<const PHINode *> &PhiUsers) {
+  if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(V))
+    if (GV->isExternallyInitialized())
+      GS.StoredType = GlobalStatus::StoredOnce;
+
   for (const Use &U : V->uses()) {
     const User *UR = U.getUser();
     if (const ConstantExpr *CE = dyn_cast<ConstantExpr>(UR)) {

Added: llvm/trunk/test/Transforms/GlobalOpt/externally-initialized.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/externally-initialized.ll?rev=250035&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/GlobalOpt/externally-initialized.ll (added)
+++ llvm/trunk/test/Transforms/GlobalOpt/externally-initialized.ll Mon Oct 12 08:20:52 2015
@@ -0,0 +1,37 @@
+; RUN: opt < %s -S -globalopt | FileCheck %s
+
+; This global is externally_initialized, which may modify the value between
+; it's static initializer and any code in this module being run, so the only
+; write to it cannot be merged into the static initialiser.
+; CHECK: @a = internal unnamed_addr externally_initialized global i32 undef
+ at a = internal externally_initialized global i32 undef
+
+; This global is stored to by the external initialization, so cannot be
+; constant-propagated and removed, despite the fact that there are no writes
+; to it.
+; CHECK: @b = internal unnamed_addr externally_initialized global i32 undef
+ at b = internal externally_initialized global i32 undef
+
+
+define void @foo() {
+; CHECK-LABEL: foo
+entry:
+; CHECK: store i32 42, i32* @a
+  store i32 42, i32* @a
+  ret void
+}
+define i32 @bar() {
+; CHECK-LABEL: bar
+entry:
+; CHECK: %val = load i32, i32* @a
+  %val = load i32, i32* @a
+  ret i32 %val
+}
+
+define i32 @baz() {
+; CHECK-LABEL: baz
+entry:
+; CHECK: %val = load i32, i32* @b
+  %val = load i32, i32* @b
+  ret i32 %val
+}