From fb51aac12966d07cf512e17ddae9183517d7798e Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Sun, 23 Mar 2014 04:22:31 +0000 Subject: [PATCH] Revert r204076 for now - it caused significant regressions in a number of benchmarks. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@204558 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Transforms/IPO/GlobalOpt.cpp | 125 +++++++++++------- test/Transforms/GlobalOpt/integer-bool.ll | 9 +- .../Transforms/GlobalOpt/twostore-gv-range.ll | 52 -------- 3 files changed, 82 insertions(+), 104 deletions(-) delete mode 100644 test/Transforms/GlobalOpt/twostore-gv-range.ll diff --git a/lib/Transforms/IPO/GlobalOpt.cpp b/lib/Transforms/IPO/GlobalOpt.cpp index 06b31c6eb5a..1a510cf4db5 100644 --- a/lib/Transforms/IPO/GlobalOpt.cpp +++ b/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. -/// This exposes the values to other scalar optimizations. -static bool TryToAddRangeMetadata(GlobalVariable *GV, Constant *OtherVal) { +/// can shrink the global into a boolean and select between the two values +/// whenever it is used. This exposes the values to other scalar optimizations. +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 - // the GV is an FP value, pointer or vector, don't do this optimization - // because range metadata is currently only supported on scalar integers. + // If GVElType is already i1, it is already shrunk. If the type of the GV is + // an FP value, pointer or vector, don't do this optimization because a select + // 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(U) && !isa(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 (!isa(InitVal) || !isa(OtherVal)) - return false; + // If initialized to zero and storing one into the global, we can use a cast + // instead of a select to synthesize the desired value. + bool IsOneZero = false; + if (ConstantInt *CI = dyn_cast(OtherVal)) + IsOneZero = InitVal->isNullValue() && CI->isOne(); - DEBUG(dbgs() << " *** ADDING RANGE METADATA: " << *GV); - - for (User *U : GV->users()) { - Instruction *UI = cast(U); - if (LoadInst *LI = dyn_cast(UI)) { - // If we already have a range, don't add a new one, so that GlobalOpt - // terminates. In theory, we could merge the two ranges. - if (LI->getMetadata(LLVMContext::MD_range)) - return false; - // Add range metadata to the load. Range metadata can represent multiple - // 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(InitVal)->getValue(); - APInt Max = cast(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); + while (!GV->use_empty()) { + Instruction *UI = cast(GV->user_back()); + if (StoreInst *SI = dyn_cast(UI)) { + // Change the store into a boolean store. + bool StoringOther = SI->getOperand(0) == OtherVal; + // Only do this if we weren't storing a loaded value. + Value *StoreVal; + if (StoringOther || SI->getOperand(0) == InitVal) { + StoreVal = ConstantInt::get(Type::getInt1Ty(GV->getContext()), + StoringOther); } else { - Value *Vals[] = { - ConstantInt::get(GV->getContext(), Min), - ConstantInt::get(GV->getContext(), Min1), - ConstantInt::get(GV->getContext(), Max), - ConstantInt::get(GV->getContext(), Max1), - }; - MDNode *MD = MDNode::get(LI->getContext(), Vals); - LI->setMetadata(LLVMContext::MD_range, MD); + // Otherwise, we are storing a previously loaded copy. To do this, + // change the copy from copying the original value to just copying the + // bool. + Instruction *StoredVal = cast(SI->getOperand(0)); + + // If we've already replaced the input, StoredVal will be a cast or + // select instruction. If not, it will be a load of the original + // global. + if (LoadInst *LI = dyn_cast(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(StoredVal) || isa(StoredVal)) && + "This is not a form that we understand!"); + StoreVal = StoredVal->getOperand(0); + assert(isa(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(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(GS.StoredOnceValue)) { if (GS.Ordering == NotAtomic) { - if (TryToAddRangeMetadata(GV, SOVConstant)) { + if (TryToShrinkGlobalToBoolean(GV, SOVConstant)) { ++NumShrunkToBool; return true; } diff --git a/test/Transforms/GlobalOpt/integer-bool.ll b/test/Transforms/GlobalOpt/integer-bool.ll index fc45c82935d..abf5fdd2ef3 100644 --- a/test/Transforms/GlobalOpt/integer-bool.ll +++ b/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 -;; such that instcombine can optimize accordingly. +;; check that global opt turns integers that only hold 0 or 1 into bools. @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 } diff --git a/test/Transforms/GlobalOpt/twostore-gv-range.ll b/test/Transforms/GlobalOpt/twostore-gv-range.ll deleted file mode 100644 index 84629ddb71a..00000000000 --- a/test/Transforms/GlobalOpt/twostore-gv-range.ll +++ /dev/null @@ -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} -- 2.34.1