<div dir="ltr">If you guarantee you are only inserting at end of block, LGTM</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 1, 2016 at 10:17 AM, Sebastian Pop <span dir="ltr"><<a href="mailto:sebpop@gmail.com" target="_blank">sebpop@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sebpop created this revision.<br>
sebpop added a reviewer: dberlin.<br>
sebpop added a subscriber: llvm-commits.<br>
<br>
With this patch we compute the DFS numbers of instructions only once and update them during the code generation when an instruction gets hoisted.<br>
Passes check and test-suite on x86_64-linux.<br>
<br>
<a href="https://reviews.llvm.org/D23021" rel="noreferrer" target="_blank">https://reviews.llvm.org/D23021</a><br>
<br>
Files:<br>
  llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
<br>
Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
===================================================================<br>
--- llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
+++ llvm/lib/Transforms/Scalar/GVNHoist.cpp<br>
@@ -72,8 +72,16 @@<br>
     //<br>
     // assert(A != B);<br>
<br>
-    unsigned ADFS = DFSNumber.lookup(A);<br>
-    unsigned BDFS = DFSNumber.lookup(B);<br>
+    const BasicBlock *BA = A->getParent();<br>
+    const BasicBlock *BB = B->getParent();<br>
+    unsigned ADFS, BDFS;<br>
+    if (BA == BB) {<br>
+      ADFS = DFSNumber.lookup(A);<br>
+      BDFS = DFSNumber.lookup(B);<br>
+    } else {<br>
+      ADFS = DFSNumber.lookup(BA);<br>
+      BDFS = DFSNumber.lookup(BB);<br>
+    }<br>
     assert (ADFS && BDFS);<br>
     return ADFS < BDFS;<br>
   }<br>
@@ -195,15 +203,17 @@<br>
     bool Res = false;<br>
     MemorySSA M(F, AA, DT);<br>
     MSSA = &M;<br>
+    // Perform DFS Numbering of instructions.<br>
+    unsigned BBI = 0;<br>
+    for (const BasicBlock *BB : depth_first(&F.getEntryBlock())) {<br>
+      DFSNumber[BB] = ++BBI;<br>
+      unsigned I = 0;<br>
+      for (auto &Inst: *BB)<br>
+        DFSNumber[&Inst] = ++I;<br>
+    }<br>
<br>
     // FIXME: use lazy evaluation of VN to avoid the fix-point computation.<br>
     while (1) {<br>
-      // Perform DFS Numbering of instructions.<br>
-      unsigned I = 0;<br>
-      for (const BasicBlock *BB : depth_first(&F.getEntryBlock()))<br>
-        for (auto &Inst: *BB)<br>
-          DFSNumber.insert({&Inst, ++I});<br>
-<br>
       auto HoistStat = hoistExpressions(F);<br>
       if (HoistStat.first + HoistStat.second == 0)<br>
         return Res;<br>
@@ -215,9 +225,6 @@<br>
         VN.clear();<br>
<br>
       Res = true;<br>
-<br>
-      // DFS numbers change when instructions are hoisted: clear and recompute.<br>
-      DFSNumber.clear();<br>
     }<br>
<br>
     return Res;<br>
@@ -741,7 +748,10 @@<br>
           continue;<br>
<br>
         // Move the instruction at the end of HoistPt.<br>
-        Repl->moveBefore(HoistPt->getTerminator());<br>
+        Instruction *Last = HoistPt->getTerminator();<br>
+        Repl->moveBefore(Last);<br>
+<br>
+        DFSNumber[Repl] = DFSNumber[Last]++;<br>
       }<br>
<br>
       MemoryAccess *NewMemAcc = nullptr;<br>
<br>
<br>
</blockquote></div><br></div>