[PATCH] D23021: GVN-hoist: compute DFS numbers once

[Sebastian Pop via llvm-commits]

I think this already does what you asked for:

 Repl->moveBefore(Last);
 DFSNumber[Repl] = DFSNumber[Last]++;

Last is the branch instruction, and we move Repl just before it, so it
will get the DFSNumber of the branch instruction,
and then we have the post increment that will update the DFSNumber of
the branch.

On Mon, Aug 1, 2016 at 12:39 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> Actually, this is very slightly wrong, you will give the wrong order for the
> terminator and the instruction before it.
> You should increase terminator number by 1, and give your inst the old
> terminator number.
>
> On Mon, Aug 1, 2016 at 10:38 AM, Daniel Berlin <dberlin at dberlin.org> wrote:
>>
>> If you guarantee you are only inserting at end of block, LGTM
>>
>> On Mon, Aug 1, 2016 at 10:17 AM, Sebastian Pop <sebpop at gmail.com> wrote:
>>>
>>> sebpop created this revision.
>>> sebpop added a reviewer: dberlin.
>>> sebpop added a subscriber: llvm-commits.
>>>
>>> With this patch we compute the DFS numbers of instructions only once and
>>> update them during the code generation when an instruction gets hoisted.
>>> Passes check and test-suite on x86_64-linux.
>>>
>>> https://reviews.llvm.org/D23021
>>>
>>> Files:
>>>   llvm/lib/Transforms/Scalar/GVNHoist.cpp
>>>
>>> Index: llvm/lib/Transforms/Scalar/GVNHoist.cpp
>>> ===================================================================
>>> --- llvm/lib/Transforms/Scalar/GVNHoist.cpp
>>> +++ llvm/lib/Transforms/Scalar/GVNHoist.cpp
>>> @@ -72,8 +72,16 @@
>>>      //
>>>      // assert(A != B);
>>>
>>> -    unsigned ADFS = DFSNumber.lookup(A);
>>> -    unsigned BDFS = DFSNumber.lookup(B);
>>> +    const BasicBlock *BA = A->getParent();
>>> +    const BasicBlock *BB = B->getParent();
>>> +    unsigned ADFS, BDFS;
>>> +    if (BA == BB) {
>>> +      ADFS = DFSNumber.lookup(A);
>>> +      BDFS = DFSNumber.lookup(B);
>>> +    } else {
>>> +      ADFS = DFSNumber.lookup(BA);
>>> +      BDFS = DFSNumber.lookup(BB);
>>> +    }
>>>      assert (ADFS && BDFS);
>>>      return ADFS < BDFS;
>>>    }
>>> @@ -195,15 +203,17 @@
>>>      bool Res = false;
>>>      MemorySSA M(F, AA, DT);
>>>      MSSA = &M;
>>> +    // Perform DFS Numbering of instructions.
>>> +    unsigned BBI = 0;
>>> +    for (const BasicBlock *BB : depth_first(&F.getEntryBlock())) {
>>> +      DFSNumber[BB] = ++BBI;
>>> +      unsigned I = 0;
>>> +      for (auto &Inst: *BB)
>>> +        DFSNumber[&Inst] = ++I;
>>> +    }
>>>
>>>      // FIXME: use lazy evaluation of VN to avoid the fix-point
>>> computation.
>>>      while (1) {
>>> -      // Perform DFS Numbering of instructions.
>>> -      unsigned I = 0;
>>> -      for (const BasicBlock *BB : depth_first(&F.getEntryBlock()))
>>> -        for (auto &Inst: *BB)
>>> -          DFSNumber.insert({&Inst, ++I});
>>> -
>>>        auto HoistStat = hoistExpressions(F);
>>>        if (HoistStat.first + HoistStat.second == 0)
>>>          return Res;
>>> @@ -215,9 +225,6 @@
>>>          VN.clear();
>>>
>>>        Res = true;
>>> -
>>> -      // DFS numbers change when instructions are hoisted: clear and
>>> recompute.
>>> -      DFSNumber.clear();
>>>      }
>>>
>>>      return Res;
>>> @@ -741,7 +748,10 @@
>>>            continue;
>>>
>>>          // Move the instruction at the end of HoistPt.
>>> -        Repl->moveBefore(HoistPt->getTerminator());
>>> +        Instruction *Last = HoistPt->getTerminator();
>>> +        Repl->moveBefore(Last);
>>> +
>>> +        DFSNumber[Repl] = DFSNumber[Last]++;
>>>        }
>>>
>>>        MemoryAccess *NewMemAcc = nullptr;
>>>
>>>
>>
>