Yes, I wasn't thinking straight ,please feel free to commits<br><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 1, 2016, 10:51 AM Sebastian Pop <<a href="mailto:sebpop@gmail.com">sebpop@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think this already does what you asked for:<br>
<br>
 Repl->moveBefore(Last);<br>
 DFSNumber[Repl] = DFSNumber[Last]++;<br>
<br>
Last is the branch instruction, and we move Repl just before it, so it<br>
will get the DFSNumber of the branch instruction,<br>
and then we have the post increment that will update the DFSNumber of<br>
the branch.<br>
<br>
On Mon, Aug 1, 2016 at 12:39 PM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>> wrote:<br>
> Actually, this is very slightly wrong, you will give the wrong order for the<br>
> terminator and the instruction before it.<br>
> You should increase terminator number by 1, and give your inst the old<br>
> terminator number.<br>
><br>
> On Mon, Aug 1, 2016 at 10:38 AM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org" target="_blank">dberlin@dberlin.org</a>> wrote:<br>
>><br>
>> If you guarantee you are only inserting at end of block, LGTM<br>
>><br>
>> On Mon, Aug 1, 2016 at 10:17 AM, Sebastian Pop <<a href="mailto:sebpop@gmail.com" target="_blank">sebpop@gmail.com</a>> wrote:<br>
>>><br>
>>> 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<br>
>>> 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<br>
>>> 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<br>
>>> 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>
>><br>
><br>
</blockquote></div>