<div dir="ltr">OK, I confirmed it was, and reverted it in r<span style="color:rgb(0,0,0)">279313. I'd get you the test case, but I've gotta run. You can probably reconstruct a test from the above verifier failure message I sent.</span></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 19, 2016 at 1:18 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I haven't actually confirmed that this change is to blame, but I strongly suspect it is.</div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 19, 2016 at 1:18 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>This makes direct intrinsic calls indirect in a diamond:</div><div><br></div><div>FAILED: obj/chrome/browser/ui/ui/bookm<wbr>ark_bar_view.o</div><div>...</div><div>Cannot take the address of an intrinsic!</div><div>  %llvm.ceil.f32.sink = select i1 %call6, float (float)* @llvm.floor.f32, float (float)* @llvm.ceil.f32, !dbg !38612</div><div>fatal error: error in backend: Broken function found, compilation aborted!</div><div>clang-4.0: error: clang frontend command failed with exit code 70 (use -v to see invocation)</div><div>clang version 4.0.0 (trunk 279289)</div></div><div class="m_4937704233376098638HOEnZb"><div class="m_4937704233376098638h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 19, 2016 at 3:10 AM, James Molloy via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jamesm<br>
Date: Fri Aug 19 05:10:27 2016<br>
New Revision: 279229<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=279229&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=279229&view=rev</a><br>
Log:<br>
[SimplifyCFG] Rewrite SinkThenElseCodeToEnd<br>
<br>
The new version has several advantages:<br>
  1) IMSHO it's more readable and neater<br>
  2) It handles loads and stores properly<br>
  3) It can handle any number of incoming blocks rather than just two. I'll be taking advantage of this in a followup patch.<br>
<br>
With this change we can now finally sink load-modify-store idioms such as:<br>
<br>
    if (a)<br>
      return *b += 3;<br>
    else<br>
      return *b += 4;<br>
<br>
    =><br>
<br>
    %z = load i32, i32* %y<br>
    %.sink = select i1 %a, i32 5, i32 7<br>
    %b = add i32 %z, %.sink<br>
    store i32 %b, i32* %y<br>
    ret i32 %b<br>
<br>
When this works for switches it'll be even more powerful.<br>
<br>
Modified:<br>
    llvm/trunk/lib/Transforms/Util<wbr>s/SimplifyCFG.cpp<br>
    llvm/trunk/test/CodeGen/ARM/av<wbr>oid-cpsr-rmw.ll<br>
    llvm/trunk/test/DebugInfo/ARM/<wbr>single-constant-use-preserves-<wbr>dbgloc.ll<br>
    llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/AArch64/prefer-fma.ll<br>
    llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/sink-common-code.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Util<wbr>s/SimplifyCFG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=279229&r1=279228&r2=279229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Utils/SimplifyCFG.cpp?rev=27<wbr>9229&r1=279228&r2=279229&view=<wbr>diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Transforms/Util<wbr>s/SimplifyCFG.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Util<wbr>s/SimplifyCFG.cpp Fri Aug 19 05:10:27 2016<br>
@@ -1319,172 +1319,232 @@ HoistTerminator:<br>
   return true;<br>
 }<br>
<br>
+// Return true if V0 and V1 are equivalent. This handles the obvious cases<br>
+// where V0 == V1 and V0 and V1 are both identical instructions, but also<br>
+// handles loads and stores with identical operands.<br>
+//<br>
+// Because determining if two memory instructions are equivalent<br>
+// depends on control flow, the \c At0 and \c At1 parameters specify a<br>
+// location for the query. This function is essentially answering the<br>
+// query "If V0 were moved to At0, and V1 were moved to At1, are V0 and V1<br>
+// equivalent?". In practice this means checking that moving V0 to At0<br>
+// doesn't cross any other memory instructions.<br>
+static bool areValuesTriviallySame(Value *V0, BasicBlock::const_iterator At0,<br>
+                                   Value *V1, BasicBlock::const_iterator At1) {<br>
+  if (V0 == V1)<br>
+    return true;<br>
+<br>
+  // Also check for instructions that are identical but not pointer-identical.<br>
+  // This can include load instructions that haven't been CSE'd.<br>
+  if (!isa<Instruction>(V0) || !isa<Instruction>(V1))<br>
+    return false;<br>
+  const auto *I0 = cast<Instruction>(V0);<br>
+  const auto *I1 = cast<Instruction>(V1);<br>
+  if (!I0->isIdenticalToWhenDefined<wbr>(I1))<br>
+    return false;<br>
+<br>
+  if (!I0->mayReadOrWriteMemory())<br>
+    return true;<br>
+<br>
+  // Instructions that may read or write memory have extra restrictions. We<br>
+  // must ensure we don't treat %a and %b as equivalent in code such as:<br>
+  //<br>
+  //  %a = load %x<br>
+  //  store %x, 1<br>
+  //  if (%c) {<br>
+  //    %b = load %x<br>
+  //    %d = add %b, 1<br>
+  //  } else {<br>
+  //    %d = add %a, 1<br>
+  //  }<br>
+<br>
+  // Be conservative. We don't want to search the entire CFG between def<br>
+  // and use; if the def isn't in the same block as the use just bail.<br>
+  if (I0->getParent() != At0->getParent() ||<br>
+      I1->getParent() != At1->getParent())<br>
+    return false;<br>
+<br>
+  // Again, be super conservative. Ideally we'd be able to query AliasAnalysis<br>
+  // but we currently don't have that available.<br>
+  auto WritesMemory = [](const Instruction &I) {<br>
+    return I.mayReadOrWriteMemory();<br>
+  };<br>
+  if (std::any_of(std::next(I0->get<wbr>Iterator()), At0, WritesMemory))<br>
+    return false;<br>
+  if (std::any_of(std::next(I1->get<wbr>Iterator()), At1, WritesMemory))<br>
+    return false;<br>
+  return true;<br>
+}<br>
+<br>
+// Is it legal to replace the operand \c OpIdx of \c GEP with a PHI node?<br>
+static bool canReplaceGEPOperandWithPHI(co<wbr>nst Instruction *GEP,<br>
+                                        unsigned OpIdx) {<br>
+  if (OpIdx == 0)<br>
+    return true;<br>
+  gep_type_iterator It = std::next(gep_type_begin(GEP), OpIdx - 1);<br>
+  return !It->isStructTy();<br>
+}<br>
+<br>
+// All blocks in Blocks unconditionally jump to a common successor. Analyze<br>
+// the last non-terminator instruction in each block and return true if it would<br>
+// be possible to sink them into their successor, creating one common<br>
+// instruction instead. Set NumPHIsRequired to the number of PHI nodes that<br>
+// would need to be created during sinking.<br>
+static bool canSinkLastInstruction(ArrayRe<wbr>f<BasicBlock*> Blocks,<br>
+                                   unsigned &NumPHIsRequired) {<br>
+  SmallVector<Instruction*,4> Insts;<br>
+  for (auto *BB : Blocks) {<br>
+    if (BB->getTerminator() == &BB->front())<br>
+      // Block was empty.<br>
+      return false;<br>
+    Insts.push_back(BB->getTermina<wbr>tor()->getPrevNode());<br>
+  }<br>
+<br>
+  // Prune out obviously bad instructions to move. Any non-store instruction<br>
+  // must have exactly one use, and we check later that use is by a single,<br>
+  // common PHI instruction in the successor.<br>
+  for (auto *I : Insts) {<br>
+    // These instructions may change or break semantics if moved.<br>
+    if (isa<PHINode>(I) || I->isEHPad() || isa<AllocaInst>(I) ||<br>
+        I->getType()->isTokenTy())<br>
+      return false;<br>
+    // Apart from loads and stores, we won't move anything that could<br>
+    // change memory or have sideeffects.<br>
+    if (!isa<StoreInst>(I) && !isa<LoadInst>(I) &&<br>
+        (I->mayHaveSideEffects() || I->mayHaveSideEffects()))<br>
+      return false;<br>
+    // Everything must have only one use too, apart from stores which<br>
+    // have no uses.<br>
+    if (!isa<StoreInst>(I) && !I->hasOneUse())<br>
+      return false;<br>
+  }<br>
+<br>
+  const Instruction *I0 = Insts.front();<br>
+  for (auto *I : Insts)<br>
+    if (!I->isSameOperationAs(I0))<br>
+      return false;<br>
+<br>
+  // If this isn't a store, check the only user is a single PHI.<br>
+  if (!isa<StoreInst>(I0)) {<br>
+    auto *PNUse = dyn_cast<PHINode>(*I0->user_be<wbr>gin());<br>
+    if (!PNUse ||<br>
+        !all_of(Insts, [&PNUse](const Instruction *I) {<br>
+          return *I->user_begin() == PNUse;<br>
+        }))<br>
+      return false;<br>
+  }<br>
+<br>
+  NumPHIsRequired = 0;<br>
+  for (unsigned OI = 0, OE = I0->getNumOperands(); OI != OE; ++OI) {<br>
+    if (I0->getOperand(OI)->getType()<wbr>->isTokenTy())<br>
+      // Don't touch any operand of token type.<br>
+      return false;<br>
+    auto SameAsI0 = [&I0, OI](const Instruction *I) {<br>
+      return areValuesTriviallySame(I->getO<wbr>perand(OI), I->getIterator(),<br>
+                                    I0->getOperand(OI), I0->getIterator());<br>
+    };<br>
+    if (!all_of(Insts, SameAsI0)) {<br>
+      if (isa<GetElementPtrInst>(I0) && !canReplaceGEPOperandWithPHI(I<wbr>0, OI))<br>
+        // We can't create a PHI from this GEP.<br>
+        return false;<br>
+      if (isa<ShuffleVectorInst>(I0) && OI == 2)<br>
+        // We can't create a PHI for a shufflevector mask.<br>
+        return false;<br>
+      ++NumPHIsRequired;<br>
+    }<br>
+  }<br>
+  return true;<br>
+}<br>
+<br>
+// Assuming canSinkLastInstruction(Blocks) has returned true, sink the last<br>
+// instruction of every block in Blocks to their common successor, commoning<br>
+// into one instruction.<br>
+static void sinkLastInstruction(ArrayRef<B<wbr>asicBlock*> Blocks) {<br>
+  unsigned Dummy;<br>
+  (void)Dummy;<br>
+  assert(canSinkLastInstruction(<wbr>Blocks, Dummy) &&<br>
+         "Must analyze before transforming!");<br>
+  auto *BBEnd = Blocks[0]->getTerminator()->ge<wbr>tSuccessor(0);<br>
+<br>
+  // canSinkLastInstruction returning true guarantees that every block has at<br>
+  // least one non-terminator instruction.<br>
+  SmallVector<Instruction*,4> Insts;<br>
+  for (auto *BB : Blocks)<br>
+    Insts.push_back(BB->getTermina<wbr>tor()->getPrevNode());<br>
+<br>
+  // We don't need to do any checking here; canSinkLastInstruction should have<br>
+  // done it all for us.<br>
+  Instruction *I0 = Insts.front();<br>
+  SmallVector<Value*, 4> NewOperands;<br>
+  for (unsigned O = 0, E = I0->getNumOperands(); O != E; ++O) {<br>
+    // This check is different to that in canSinkLastInstruction. There, we<br>
+    // cared about the global view once simplifycfg (and instcombine) have<br>
+    // completed - it takes into account PHIs that become trivially<br>
+    // simplifiable.  However here we need a more local view; if an operand<br>
+    // differs we create a PHI and rely on instcombine to clean up the very<br>
+    // small mess we may make.<br>
+    bool NeedPHI = any_of(Insts, [&I0, O](const Instruction *I) {<br>
+      return I->getOperand(O) != I0->getOperand(O);<br>
+    });<br>
+    if (!NeedPHI) {<br>
+      NewOperands.push_back(I0->getO<wbr>perand(O));<br>
+      continue;<br>
+    }<br>
+<br>
+    // Create a new PHI in the successor block and populate it.<br>
+    auto *Op = I0->getOperand(O);<br>
+    assert(!Op->getType()->isToken<wbr>Ty() && "Can't PHI tokens!");<br>
+    auto *PN = PHINode::Create(Op->getType(), Insts.size(),<br>
+                               Op->getName() + ".sink", &BBEnd->front());<br>
+    for (auto *I : Insts)<br>
+      PN->addIncoming(I->getOperand(<wbr>O), I->getParent());<br>
+    NewOperands.push_back(PN);<br>
+  }<br>
+<br>
+  // Arbitrarily use I0 as the new "common" instruction; remap its operands<br>
+  // and move it to the start of the successor block.<br>
+  for (unsigned O = 0, E = I0->getNumOperands(); O != E; ++O)<br>
+    I0->getOperandUse(O).set(NewOp<wbr>erands[O]);<br>
+  I0->moveBefore(&*BBEnd->getFir<wbr>stInsertionPt());<br>
+<br>
+  if (!isa<StoreInst>(I0)) {<br>
+    // canSinkLastInstruction checked that all instructions were used by<br>
+    // one and only one PHI node. Find that now, RAUW it to our common<br>
+    // instruction and nuke it.<br>
+    assert(I0->hasOneUse());<br>
+    auto *PN = cast<PHINode>(*I0->user_begin(<wbr>));<br>
+    PN->replaceAllUsesWith(I0);<br>
+    PN->eraseFromParent();<br>
+  }<br>
+<br>
+  // Finally nuke all instructions apart from the common instruction.<br>
+  for (auto *I : Insts)<br>
+    if (I != I0)<br>
+      I->eraseFromParent();<br>
+}<br>
+<br>
 /// Given an unconditional branch that goes to BBEnd,<br>
 /// check whether BBEnd has only two predecessors and the other predecessor<br>
 /// ends with an unconditional branch. If it is true, sink any common code<br>
 /// in the two predecessors to BBEnd.<br>
 static bool SinkThenElseCodeToEnd(BranchIn<wbr>st *BI1) {<br>
   assert(BI1->isUnconditional()<wbr>);<br>
-  BasicBlock *BB1 = BI1->getParent();<br>
   BasicBlock *BBEnd = BI1->getSuccessor(0);<br>
<br>
-  // Check that BBEnd has two predecessors and the other predecessor ends with<br>
-  // an unconditional branch.<br>
-  pred_iterator PI = pred_begin(BBEnd), PE = pred_end(BBEnd);<br>
-  BasicBlock *Pred0 = *PI++;<br>
-  if (PI == PE) // Only one predecessor.<br>
-    return false;<br>
-  BasicBlock *Pred1 = *PI++;<br>
-  if (PI != PE) // More than two predecessors.<br>
-    return false;<br>
-  BasicBlock *BB2 = (Pred0 == BB1) ? Pred1 : Pred0;<br>
-  BranchInst *BI2 = dyn_cast<BranchInst>(BB2->getT<wbr>erminator());<br>
-  if (!BI2 || !BI2->isUnconditional())<br>
-    return false;<br>
-<br>
-  // Gather the PHI nodes in BBEnd.<br>
-  SmallDenseMap<std::pair<Value *, Value *>, PHINode *> JointValueMap;<br>
-  Instruction *FirstNonPhiInBBEnd = nullptr;<br>
-  for (BasicBlock::iterator I = BBEnd->begin(), E = BBEnd->end(); I != E; ++I) {<br>
-    if (PHINode *PN = dyn_cast<PHINode>(I)) {<br>
-      Value *BB1V = PN->getIncomingValueForBlock(B<wbr>B1);<br>
-      Value *BB2V = PN->getIncomingValueForBlock(B<wbr>B2);<br>
-      JointValueMap[std::make_pair(B<wbr>B1V, BB2V)] = PN;<br>
-    } else {<br>
-      FirstNonPhiInBBEnd = &*I;<br>
-      break;<br>
-    }<br>
-  }<br>
-  if (!FirstNonPhiInBBEnd)<br>
+  SmallVector<BasicBlock*,4> Blocks;<br>
+  for (auto *BB : predecessors(BBEnd))<br>
+    Blocks.push_back(BB);<br>
+  if (Blocks.size() != 2 ||<br>
+      !all_of(Blocks, [](const BasicBlock *BB) {<br>
+        auto *BI = dyn_cast<BranchInst>(BB->getTe<wbr>rminator());<br>
+        return BI && BI->isUnconditional();<br>
+      }))<br>
     return false;<br>
<br>
-  // This does very trivial matching, with limited scanning, to find identical<br>
-  // instructions in the two blocks.  We scan backward for obviously identical<br>
-  // instructions in an identical order.<br>
-  BasicBlock::InstListType::reve<wbr>rse_iterator RI1 = BB1->getInstList().rbegin(),<br>
-                                             RE1 = BB1->getInstList().rend(),<br>
-                                             RI2 = BB2->getInstList().rbegin(),<br>
-                                             RE2 = BB2->getInstList().rend();<br>
-  // Skip debug info.<br>
-  while (RI1 != RE1 && isa<DbgInfoIntrinsic>(&*RI1))<br>
-    ++RI1;<br>
-  if (RI1 == RE1)<br>
-    return false;<br>
-  while (RI2 != RE2 && isa<DbgInfoIntrinsic>(&*RI2))<br>
-    ++RI2;<br>
-  if (RI2 == RE2)<br>
-    return false;<br>
-  // Skip the unconditional branches.<br>
-  ++RI1;<br>
-  ++RI2;<br>
-<br>
   bool Changed = false;<br>
-  while (RI1 != RE1 && RI2 != RE2) {<br>
-    // Skip debug info.<br>
-    while (RI1 != RE1 && isa<DbgInfoIntrinsic>(&*RI1))<br>
-      ++RI1;<br>
-    if (RI1 == RE1)<br>
-      return Changed;<br>
-    while (RI2 != RE2 && isa<DbgInfoIntrinsic>(&*RI2))<br>
-      ++RI2;<br>
-    if (RI2 == RE2)<br>
-      return Changed;<br>
-<br>
-    Instruction *I1 = &*RI1, *I2 = &*RI2;<br>
-    auto InstPair = std::make_pair(I1, I2);<br>
-    // I1 and I2 should have a single use in the same PHI node, and they<br>
-    // perform the same operation.<br>
-    // Cannot move control-flow-involving, volatile loads, vaarg, etc.<br>
-    if (isa<PHINode>(I1) || isa<PHINode>(I2) || isa<TerminatorInst>(I1) ||<br>
-        isa<TerminatorInst>(I2) || I1->isEHPad() || I2->isEHPad() ||<br>
-        isa<AllocaInst>(I1) || isa<AllocaInst>(I2) ||<br>
-        I1->mayHaveSideEffects() || I2->mayHaveSideEffects() ||<br>
-        I1->mayReadOrWriteMemory() || I2->mayReadOrWriteMemory() ||<br>
-        !I1->hasOneUse() || !I2->hasOneUse() || !JointValueMap.count(InstPair)<wbr>)<br>
-      return Changed;<br>
-<br>
-    // Check whether we should swap the operands of ICmpInst.<br>
-    // TODO: Add support of communativity.<br>
-    ICmpInst *ICmp1 = dyn_cast<ICmpInst>(I1), *ICmp2 = dyn_cast<ICmpInst>(I2);<br>
-    bool SwapOpnds = false;<br>
-    if (ICmp1 && ICmp2 && ICmp1->getOperand(0) != ICmp2->getOperand(0) &&<br>
-        ICmp1->getOperand(1) != ICmp2->getOperand(1) &&<br>
-        (ICmp1->getOperand(0) == ICmp2->getOperand(1) ||<br>
-         ICmp1->getOperand(1) == ICmp2->getOperand(0))) {<br>
-      ICmp2->swapOperands();<br>
-      SwapOpnds = true;<br>
-    }<br>
-    if (!I1->isSameOperationAs(I2)) {<br>
-      if (SwapOpnds)<br>
-        ICmp2->swapOperands();<br>
-      return Changed;<br>
-    }<br>
-<br>
-    // The operands should be either the same or they need to be generated<br>
-    // with a PHI node after sinking. We only handle the case where there is<br>
-    // a single pair of different operands.<br>
-    Value *DifferentOp1 = nullptr, *DifferentOp2 = nullptr;<br>
-    unsigned Op1Idx = ~0U;<br>
-    for (unsigned I = 0, E = I1->getNumOperands(); I != E; ++I) {<br>
-      if (I1->getOperand(I) == I2->getOperand(I))<br>
-        continue;<br>
-      // Early exit if we have more-than one pair of different operands or if<br>
-      // we need a PHI node to replace a constant.<br>
-      if (Op1Idx != ~0U || isa<Constant>(I1->getOperand(I<wbr>)) ||<br>
-          isa<Constant>(I2->getOperand(I<wbr>))) {<br>
-        // If we can't sink the instructions, undo the swapping.<br>
-        if (SwapOpnds)<br>
-          ICmp2->swapOperands();<br>
-        return Changed;<br>
-      }<br>
-      DifferentOp1 = I1->getOperand(I);<br>
-      Op1Idx = I;<br>
-      DifferentOp2 = I2->getOperand(I);<br>
-    }<br>
-<br>
-    DEBUG(dbgs() << "SINK common instructions " << *I1 << "\n");<br>
-    DEBUG(dbgs() << "                         " << *I2 << "\n");<br>
-<br>
-    // We insert the pair of different operands to JointValueMap and<br>
-    // remove (I1, I2) from JointValueMap.<br>
-    if (Op1Idx != ~0U) {<br>
-      auto &NewPN = JointValueMap[std::make_pair(D<wbr>ifferentOp1, DifferentOp2)];<br>
-      if (!NewPN) {<br>
-        NewPN =<br>
-            PHINode::Create(DifferentOp1-><wbr>getType(), 2,<br>
-                            DifferentOp1->getName() + ".sink", &BBEnd->front());<br>
-        NewPN->addIncoming(DifferentOp<wbr>1, BB1);<br>
-        NewPN->addIncoming(DifferentOp<wbr>2, BB2);<br>
-        DEBUG(dbgs() << "Create PHI node " << *NewPN << "\n";);<br>
-      }<br>
-      // I1 should use NewPN instead of DifferentOp1.<br>
-      I1->setOperand(Op1Idx, NewPN);<br>
-    }<br>
-    PHINode *OldPN = JointValueMap[InstPair];<br>
-    JointValueMap.erase(InstPair);<br>
-<br>
-    // We need to update RE1 and RE2 if we are going to sink the first<br>
-    // instruction in the basic block down.<br>
-    bool UpdateRE1 = (I1 == &BB1->front()), UpdateRE2 = (I2 == &BB2->front());<br>
-    // Sink the instruction.<br>
-    BBEnd->getInstList().splice(Fi<wbr>rstNonPhiInBBEnd->getIterator(<wbr>),<br>
-                                BB1->getInstList(), I1);<br>
-    if (!OldPN->use_empty())<br>
-      OldPN->replaceAllUsesWith(I1);<br>
-    OldPN->eraseFromParent();<br>
-<br>
-    if (!I2->use_empty())<br>
-      I2->replaceAllUsesWith(I1);<br>
-    I1->intersectOptionalDataWith(<wbr>I2);<br>
-    // TODO: Use combineMetadata here to preserve what metadata we can<br>
-    // (analogous to the hoisting case above).<br>
-    I2->eraseFromParent();<br>
-<br>
-    if (UpdateRE1)<br>
-      RE1 = BB1->getInstList().rend();<br>
-    if (UpdateRE2)<br>
-      RE2 = BB2->getInstList().rend();<br>
-    FirstNonPhiInBBEnd = &*I1;<br>
+  unsigned NumPHIsToInsert;<br>
+  while (canSinkLastInstruction(Blocks<wbr>, NumPHIsToInsert) && NumPHIsToInsert <= 1) {<br>
+    sinkLastInstruction(Blocks);<br>
     NumSinkCommons++;<br>
     Changed = true;<br>
   }<br>
<br>
Modified: llvm/trunk/test/CodeGen/ARM/av<wbr>oid-cpsr-rmw.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/avoid-cpsr-rmw.ll?rev=279229&r1=279228&r2=279229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/CodeGen/<wbr>ARM/avoid-cpsr-rmw.ll?rev=2792<wbr>29&r1=279228&r2=279229&view=<wbr>diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/ARM/av<wbr>oid-cpsr-rmw.ll (original)<br>
+++ llvm/trunk/test/CodeGen/ARM/av<wbr>oid-cpsr-rmw.ll Fri Aug 19 05:10:27 2016<br>
@@ -106,7 +106,7 @@ if.then:<br>
<br>
 if.else:<br>
   store i32 3, i32* %p, align 4<br>
-  %incdec.ptr5 = getelementptr inbounds i32, i32* %p, i32 2<br>
+  %incdec.ptr5 = getelementptr inbounds i32, i32* %p, i32 3<br>
   store i32 5, i32* %incdec.ptr1, align 4<br>
   store i32 6, i32* %incdec.ptr5, align 4<br>
   br label %if.end<br>
<br>
Modified: llvm/trunk/test/DebugInfo/ARM/<wbr>single-constant-use-preserves-<wbr>dbgloc.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll?rev=279229&r1=279228&r2=279229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/DebugInf<wbr>o/ARM/single-constant-use-pres<wbr>erves-dbgloc.ll?rev=279229&r1=<wbr>279228&r2=279229&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/DebugInfo/ARM/<wbr>single-constant-use-preserves-<wbr>dbgloc.ll (original)<br>
+++ llvm/trunk/test/DebugInfo/ARM/<wbr>single-constant-use-preserves-<wbr>dbgloc.ll Fri Aug 19 05:10:27 2016<br>
@@ -9,8 +9,9 @@<br>
 ;     return -1;<br>
 ; }<br>
<br>
+; CHECK: mvnlt<br>
 ; CHECK: .loc 1 6 7<br>
-; CHECK: mvn<br>
+; CHECK: strlt<br>
<br>
 target datalayout = "e-m:e-p:32:32-i64:64-v128:64:<wbr>128-a:0:32-n32-S64"<br>
 target triple = "armv7--linux-gnueabihf"<br>
<br>
Modified: llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/AArch64/prefer-fma.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/AArch64/prefer-fma.ll?rev=279229&r1=279228&r2=279229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/SimplifyCFG/AArch64/prefer-<wbr>fma.ll?rev=279229&r1=279228&r2<wbr>=279229&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/AArch64/prefer-fma.ll (original)<br>
+++ llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/AArch64/prefer-fma.ll Fri Aug 19 05:10:27 2016<br>
@@ -29,7 +29,8 @@ if.else:<br>
   %4 = load double, double* %a, align 8<br>
   %mul1 = fmul fast double %1, %4<br>
   %sub1 = fsub fast double %mul1, %0<br>
-  store double %sub1, double* %y, align 8<br>
+  %gep1 = getelementptr double, double* %y, i32 1<br>
+  store double %sub1, double* %gep1, align 8<br>
   br label %if.end<br>
<br>
 if.end:                                           ; preds = %if.else, %if.then<br>
<br>
Modified: llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/sink-common-code.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll?rev=279229&r1=279228&r2=279229&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Transfor<wbr>ms/SimplifyCFG/sink-common-cod<wbr>e.ll?rev=279229&r1=279228&r2=<wbr>279229&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/sink-common-code.ll (original)<br>
+++ llvm/trunk/test/Transforms/Sim<wbr>plifyCFG/sink-common-code.ll Fri Aug 19 05:10:27 2016<br>
@@ -81,3 +81,204 @@ if.end:<br>
 ; CHECK: call<br>
 ; CHECK: add<br>
 ; CHECK-NOT: br<br>
+<br>
+define i32 @test4(i1 zeroext %flag, i32 %x, i32* %y) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %a = add i32 %x, 5<br>
+  store i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %b = add i32 %x, 7<br>
+  store i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test4<br>
+; CHECK: select<br>
+; CHECK: store<br>
+; CHECK-NOT: store<br>
+<br>
+define i32 @test5(i1 zeroext %flag, i32 %x, i32* %y) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %a = add i32 %x, 5<br>
+  store volatile i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %b = add i32 %x, 7<br>
+  store i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test5<br>
+; CHECK: store volatile<br>
+; CHECK: store<br>
+<br>
+define i32 @test6(i1 zeroext %flag, i32 %x, i32* %y) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %a = add i32 %x, 5<br>
+  store volatile i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %b = add i32 %x, 7<br>
+  store volatile i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test6<br>
+; CHECK: select<br>
+; CHECK: store volatile<br>
+; CHECK-NOT: store<br>
+<br>
+define i32 @test7(i1 zeroext %flag, i32 %x, i32* %y) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %z = load volatile i32, i32* %y<br>
+  %a = add i32 %z, 5<br>
+  store volatile i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %w = load volatile i32, i32* %y<br>
+  %b = add i32 %w, 7<br>
+  store volatile i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test7<br>
+; CHECK-DAG: select<br>
+; CHECK-DAG: load volatile<br>
+; CHECK: store volatile<br>
+; CHECK-NOT: load<br>
+; CHECK-NOT: store<br>
+<br>
+; %z and %w are in different blocks. We shouldn't sink the add because<br>
+; there may be intervening memory instructions.<br>
+define i32 @test8(i1 zeroext %flag, i32 %x, i32* %y) {<br>
+entry:<br>
+  %z = load volatile i32, i32* %y<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %a = add i32 %z, 5<br>
+  store volatile i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %w = load volatile i32, i32* %y<br>
+  %b = add i32 %w, 7<br>
+  store volatile i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test8<br>
+; CHECK: add<br>
+; CHECK: add<br>
+<br>
+; The extra store in %if.then means %z and %w are not equivalent.<br>
+define i32 @test9(i1 zeroext %flag, i32 %x, i32* %y, i32* %p) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  store i32 7, i32* %p<br>
+  %z = load volatile i32, i32* %y<br>
+  store i32 6, i32* %p<br>
+  %a = add i32 %z, 5<br>
+  store volatile i32 %a, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %w = load volatile i32, i32* %y<br>
+  %b = add i32 %w, 7<br>
+  store volatile i32 %b, i32* %y<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test9<br>
+; CHECK: add<br>
+; CHECK: add<br>
+<br>
+%struct.anon = type { i32, i32 }<br>
+<br>
+; The GEP indexes a struct type so cannot have a variable last index.<br>
+define i32 @test10(i1 zeroext %flag, i32 %x, i32* %y, %struct.anon* %s) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %dummy = add i32 %x, 5<br>
+  %gepa = getelementptr inbounds %struct.anon, %struct.anon* %s, i32 0, i32 0<br>
+  store volatile i32 %x, i32* %gepa<br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %dummy1 = add i32 %x, 6<br>
+  %gepb = getelementptr inbounds %struct.anon, %struct.anon* %s, i32 0, i32 1<br>
+  store volatile i32 %x, i32* %gepb<br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test10<br>
+; CHECK: getelementptr<br>
+; CHECK: getelementptr<br>
+; CHECK: phi<br>
+; CHECK: store volatile<br>
+<br>
+; The shufflevector's mask operand cannot be merged in a PHI.<br>
+define i32 @test11(i1 zeroext %flag, i32 %w, <2 x i32> %x, <2 x i32> %y) {<br>
+entry:<br>
+  br i1 %flag, label %if.then, label %if.else<br>
+<br>
+if.then:<br>
+  %dummy = add i32 %w, 5<br>
+  %sv1 = shufflevector <2 x i32> %x, <2 x i32> %y, <2 x i32> <i32 0, i32 1><br>
+  br label %if.end<br>
+<br>
+if.else:<br>
+  %dummy1 = add i32 %w, 6<br>
+  %sv2 = shufflevector <2 x i32> %x, <2 x i32> %y, <2 x i32> <i32 1, i32 0><br>
+  br label %if.end<br>
+<br>
+if.end:<br>
+  %p = phi <2 x i32> [ %sv1, %if.then ], [ %sv2, %if.else ]<br>
+  ret i32 1<br>
+}<br>
+<br>
+; CHECK-LABEL: test11<br>
+; CHECK: shufflevector<br>
+; CHECK: shufflevector<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>