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

[Daniel Berlin via llvm-commits]

Yes, I wasn't thinking straight ,please feel free to commits

On Mon, Aug 1, 2016, 10:51 AM Sebastian Pop <sebpop at gmail.com> wrote:

> 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;
> >>>
> >>>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160801/74c3f15e/attachment.html>