[llvm-dev] RFC: Making a common successor/predecessor interface

[Alina Sbirlea via llvm-dev]

On Tue, Mar 10, 2020 at 9:46 AM Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> Hi Alina,
>
> On Tue, Mar 10, 2020 at 4:31 PM Alina Sbirlea via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> > Yes, I considered declaring the "global" succ_begin for the other block
> types, but it seems like a more complex change (probably to declare the
> type as Dave described, and these need adding for 3 more types vs 2 now)
> and these wouldn't be used anywhere else.
> > AFAICT, there is no issue with replacing the current "global" iterators
> with class specific ones, they are already used as such. But perhaps I
> don't have the full picture.
> >  In the first patch I put up, the iterators added inside BasicBlock can
> co-exist with the global ones, so the switch can be done incrementally.
>
> So the specific reason I was asking is that for GPU applications, we
> are interested in potentially having two CFGs overlaying the same set
> of basic blocks. Due to the SIMT nature of execution there are two
> types of control flow.
>
> On second thought, however, it probably doesn't make much of a
> difference for that particular problem whether succ_begin/end are
> global or member functions.
>
> And from other perspectives, having them as member functions seems
> slightly better because it makes it easier to find them. So in
> summary, I suppose either way is fine, as long as we're making things
> consistent :)
>

Sounds good :). Thank you for your input!

Best,
Alina

Alina

>
> Cheers,
> Nicolai
>
> >
> >
> > Thanks,
> > Alina
> >
> > On Mon, Mar 9, 2020, 4:16 PM David Blaikie <dblaikie at gmail.com> wrote:
> >>
> >>
> >>
> >> On Mon, Mar 9, 2020 at 3:57 PM Alina Sbirlea via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>>
> >>> Hi,
> >>>
> >>> As part of an ongoing work to extend the GraphDiff (this models a CFG
> view), I came across the need to have a common interface for accessing
> successors/predecessors in various IR units, such that a type such as
> `typename NodeT::succ_iterator` could be used in templated code.
> >>
> >>
> >> I /think/ this can be achieved with the existing API by using
> "decltype(succ_begin(std::declval<NodeT>()))" instead of the typename
> you've got as an example (it looks like succ_begin is the extension point -
> but the problem you're having is naming its return type? decltype would be
> one option) - you could make a trait wrapper around that or the like if you
> need this type name in a bunch of disparate places (where they can't share
> a typedef).
> >>
> >> Would that suffice? are there other aspects of your use case that don't
> line up well with the existing non-member/overload API?
> >>
> >> - Dave
> >>
> >>>
> >>> In particular, the need arose for BasicBlocks, MachineBasicBlocks,
> VPBlockBase and clang::CFGBlock.
> >>>
> >>> The least invasive change seemed to be to use the interface already
> being used in MachineBasicBlock and clang::CFGBlock, and:
> >>> (1) update BasicBlock to use this instead of the "global"
> `succ_iterator` in IR/CFG.h
> >>> (2) add the same interfaces in VPBlockBase as simple wrappers over
> existing Successors/Predecessors vectors.
> >>>
> >>> I've been working on a few patches to make this happen, but I'd like
> the community's thoughts on this before deep-diving into code reviews.
> >>>
> >>> For some concrete view of what the changes look like, I uploaded two
> preliminary patches:
> >>> (1) part 1: D75881: Introducing class specific iterators
> >>> (2) D75882
> >>> (1) part 2: pending: Cleaning up existing usages; example replacement:
> `succ_begin(BB)` with `BB->succ_begin()`.
> >>> (1) part3/4: pending: Add class specific iterators to `Instruction`
> and clean up existing usages just as for `BasicBlock`.
> >>>
> >>> I split the above (1) just to clarify what interfaces are added versus
> the NFC cleanups that follow. But it could be done just as well in a single
> patch.
> >>>
> >>> I welcome comments on this, and if there's something I missed
> explaining please let me know.
> >>>
> >>> Thank you,
> >>> Alina
> >>>
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> LLVM Developers mailing list
> >>> llvm-dev at lists.llvm.org
> >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > llvm-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
>
> --
> Lerne, wie die Welt wirklich ist,
> aber vergiss niemals, wie sie sein sollte.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200310/98c1d0f0/attachment-0001.html>