Revert r204076 for now - it caused significant regressions in a number of

benchmarks.

<rdar://problem/16368461>



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

lib/Transforms/IPO/GlobalOpt.cpp

@@ -1588,16 +1588,18 @@ static bool OptimizeOnceStoredGlobal(GlobalVariable *GV, Value *StoredOnceVal,
   return false;
 }
 
-/// TryToAddRangeMetadata - At this point, we have learned that the only
+/// TryToShrinkGlobalToBoolean - At this point, we have learned that the only
 /// two values ever stored into GV are its initializer and OtherVal.  See if we
-/// can annotate loads from it with range metadata describing this.
+/// can shrink the global into a boolean and select between the two values
-/// This exposes the values to other scalar optimizations.
+/// whenever it is used.  This exposes the values to other scalar optimizations.
-static bool TryToAddRangeMetadata(GlobalVariable *GV, Constant *OtherVal) {
+static bool TryToShrinkGlobalToBoolean(GlobalVariable *GV, Constant *OtherVal) {
   Type *GVElType = GV->getType()->getElementType();
 
-  // If GVElType is already i1, it already has a minimal range. If the type of
+  // If GVElType is already i1, it is already shrunk.  If the type of the GV is
-  // the GV is an FP value, pointer or vector, don't do this optimization
+  // an FP value, pointer or vector, don't do this optimization because a select
-  // because range metadata is currently only supported on scalar integers.
+  // between them is very expensive and unlikely to lead to later
+  // simplification.  In these cases, we typically end up with "cond ? v1 : v2"
+  // where v1 and v2 both require constant pool loads, a big loss.
   if (GVElType == Type::getInt1Ty(GV->getContext()) ||
       GVElType->isFloatingPointTy() ||
       GVElType->isPointerTy() || GVElType->isVectorTy())
@@ -1609,53 +1611,81 @@ static bool TryToAddRangeMetadata(GlobalVariable *GV, Constant *OtherVal) {
     if (!isa<LoadInst>(U) && !isa<StoreInst>(U))
       return false;
 
+  DEBUG(dbgs() << "   *** SHRINKING TO BOOL: " << *GV);
+
+  // Create the new global, initializing it to false.
+  GlobalVariable *NewGV = new GlobalVariable(Type::getInt1Ty(GV->getContext()),
+                                             false,
+                                             GlobalValue::InternalLinkage,
+                                        ConstantInt::getFalse(GV->getContext()),
+                                             GV->getName()+".b",
+                                             GV->getThreadLocalMode(),
+                                             GV->getType()->getAddressSpace());
+  GV->getParent()->getGlobalList().insert(GV, NewGV);
+
   Constant *InitVal = GV->getInitializer();
   assert(InitVal->getType() != Type::getInt1Ty(GV->getContext()) &&
-         "No reason to add range metadata!");
+         "No reason to shrink to bool!");
 
-  // The MD_range metadata only supports absolute integer constants.
+  // If initialized to zero and storing one into the global, we can use a cast
-  if (!isa<ConstantInt>(InitVal) || !isa<ConstantInt>(OtherVal))
+  // instead of a select to synthesize the desired value.
-    return false;
+  bool IsOneZero = false;
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(OtherVal))
+    IsOneZero = InitVal->isNullValue() && CI->isOne();
 
-  DEBUG(dbgs() << "   *** ADDING RANGE METADATA: " << *GV);
+  while (!GV->use_empty()) {
-
+    Instruction *UI = cast<Instruction>(GV->user_back());
-  for (User *U : GV->users()) {
+    if (StoreInst *SI = dyn_cast<StoreInst>(UI)) {
-    Instruction *UI = cast<Instruction>(U);
+      // Change the store into a boolean store.
-    if (LoadInst *LI = dyn_cast<LoadInst>(UI)) {
+      bool StoringOther = SI->getOperand(0) == OtherVal;
-      // If we already have a range, don't add a new one, so that GlobalOpt
+      // Only do this if we weren't storing a loaded value.
-      // terminates. In theory, we could merge the two ranges.
+      Value *StoreVal;
-      if (LI->getMetadata(LLVMContext::MD_range))
+      if (StoringOther || SI->getOperand(0) == InitVal) {
-        return false;
+        StoreVal = ConstantInt::get(Type::getInt1Ty(GV->getContext()),
-      // Add range metadata to the load. Range metadata can represent multiple
+                                    StoringOther);
-      // ranges, but they must be discontiguous, so we have two cases: the case
-      // where the values are adjacent, in which case we add one range, and the
-      // case where they're not, in which case we add two.
-      APInt Min = cast<ConstantInt>(InitVal)->getValue();
-      APInt Max = cast<ConstantInt>(OtherVal)->getValue();
-      if (Max.slt(Min))
-        std::swap(Min, Max);
-      APInt Min1 = Min + 1;
-      APInt Max1 = Max + 1;
-      if (Min1 == Max) {
-        Value *Vals[] = {
-          ConstantInt::get(GV->getContext(), Min),
-          ConstantInt::get(GV->getContext(), Max1),
-        };
-        MDNode *MD = MDNode::get(LI->getContext(), Vals);
-        LI->setMetadata(LLVMContext::MD_range, MD);
       } else {
-        Value *Vals[] = {
+        // Otherwise, we are storing a previously loaded copy.  To do this,
-          ConstantInt::get(GV->getContext(), Min),
+        // change the copy from copying the original value to just copying the
-          ConstantInt::get(GV->getContext(), Min1),
+        // bool.
-          ConstantInt::get(GV->getContext(), Max),
+        Instruction *StoredVal = cast<Instruction>(SI->getOperand(0));
-          ConstantInt::get(GV->getContext(), Max1),
+
-        };
+        // If we've already replaced the input, StoredVal will be a cast or
-        MDNode *MD = MDNode::get(LI->getContext(), Vals);
+        // select instruction.  If not, it will be a load of the original
-        LI->setMetadata(LLVMContext::MD_range, MD);
+        // global.
+        if (LoadInst *LI = dyn_cast<LoadInst>(StoredVal)) {
+          assert(LI->getOperand(0) == GV && "Not a copy!");
+          // Insert a new load, to preserve the saved value.
+          StoreVal = new LoadInst(NewGV, LI->getName()+".b", false, 0,
+                                  LI->getOrdering(), LI->getSynchScope(), LI);
+        } else {
+          assert((isa<CastInst>(StoredVal) || isa<SelectInst>(StoredVal)) &&
+                 "This is not a form that we understand!");
+          StoreVal = StoredVal->getOperand(0);
+          assert(isa<LoadInst>(StoreVal) && "Not a load of NewGV!");
+        }
       }
+      new StoreInst(StoreVal, NewGV, false, 0,
+                    SI->getOrdering(), SI->getSynchScope(), SI);
+    } else {
+      // Change the load into a load of bool then a select.
+      LoadInst *LI = cast<LoadInst>(UI);
+      LoadInst *NLI = new LoadInst(NewGV, LI->getName()+".b", false, 0,
+                                   LI->getOrdering(), LI->getSynchScope(), LI);
+      Value *NSI;
+      if (IsOneZero)
+        NSI = new ZExtInst(NLI, LI->getType(), "", LI);
+      else
+        NSI = SelectInst::Create(NLI, OtherVal, InitVal, "", LI);
+      NSI->takeName(LI);
+      LI->replaceAllUsesWith(NSI);
     }
+    UI->eraseFromParent();
   }
 
+  // Retain the name of the old global variable. People who are debugging their
+  // programs may expect these variables to be named the same.
+  NewGV->takeName(GV);
+  GV->eraseFromParent();
   return true;
 }
 
@@ -1809,10 +1839,11 @@ bool GlobalOpt::ProcessInternalGlobal(GlobalVariable *GV,
                                  DL, TLI))
       return true;
 
-    // Otherwise, if the global was not a boolean, we can add range metadata.
+    // Otherwise, if the global was not a boolean, we can shrink it to be a
+    // boolean.
     if (Constant *SOVConstant = dyn_cast<Constant>(GS.StoredOnceValue)) {
       if (GS.Ordering == NotAtomic) {
-        if (TryToAddRangeMetadata(GV, SOVConstant)) {
+        if (TryToShrinkGlobalToBoolean(GV, SOVConstant)) {
           ++NumShrunkToBool;
           return true;
         }

test/Transforms/GlobalOpt/integer-bool.ll

@@ -1,21 +1,20 @@
 ; RUN: opt < %s -S -globalopt -instcombine | FileCheck %s
-;; check that global opt annotates loads from global variales that only hold 0 or 1
+;; check that global opt turns integers that only hold 0 or 1 into bools.
-;; such that instcombine can optimize accordingly.
 
 @G = internal addrspace(1) global i32 0
 ; CHECK: @G
 ; CHECK: addrspace(1)
-; CHECK: global i32 0
+; CHECK: global i1 false
 
 define void @set1() {
   store i32 0, i32 addrspace(1)* @G
-; CHECK: store i32 0
+; CHECK: store i1 false
   ret void
 }
 
 define void @set2() {
   store i32 1, i32 addrspace(1)* @G
-; CHECK: store i32 1
+; CHECK: store i1 true
   ret void
 }
 

test/Transforms/GlobalOpt/twostore-gv-range.ll

@@ -1,52 +0,0 @@
-; RUN: opt < %s -S -globalopt | FileCheck %s
-;; check that global opt annotates loads from global variales that have
-;; constant values stored to them.
-
-@G = internal global i32 5
-@H = internal global i32 7
-@I = internal global i32 17
-@J = internal global i32 29
-@K = internal global i32 31
-
-define void @set() {
-  store i32 6, i32* @G
-  store i32 13, i32* @H
-  store i32 16, i32* @I
-  store i32 29, i32* @J
-  store i32 -37, i32* @K
-  ret void
-}
-
-define i32 @getG() {
-; CHECK: %t = load i32* @G, !range [[G:![0-9]+]]
-  %t = load i32* @G
-  ret i32 %t
-}
-define i32 @getH() {
-; CHECK: %t = load i32* @H, !range [[H:![0-9]+]]
-  %t = load i32* @H
-  ret i32 %t
-}
-
-define i32 @getI() {
-; CHECK: %t = load i32* @I, !range [[I:![0-9]+]]
-  %t = load i32* @I
-  ret i32 %t
-}
-
-define i32 @getJ() {
-; CHECK: ret i32 29
-  %t = load i32* @J
-  ret i32 %t
-}
-
-define i32 @getK() {
-; CHECK: %t = load i32* @K, !range [[K:![0-9]+]]
-  %t = load i32* @K
-  ret i32 %t
-}
-
-; CHECK: [[G]] = metadata !{i32 5, i32 7}
-; CHECK: [[H]] = metadata !{i32 7, i32 8, i32 13, i32 14}
-; CHECK: [[I]] = metadata !{i32 16, i32 18}
-; CHECK: [[K]] = metadata !{i32 -37, i32 -36, i32 31, i32 32}