[PATCH] D34798: [Dominators] Add CFGBuilder testing utility

[David Blaikie via llvm-commits]

On Thu, Jun 29, 2017 at 4:27 PM Jakub Kuderski via Phabricator <
reviews at reviews.llvm.org> wrote:

> kuhar added a comment.
>
> Thanks for the review, David!
>
> > Ah, hmm - I was assuming this was an API for building the CFG data
> structures. But I see it's an API for building IR that represents
> particular CFGs (so you can then test the CFG analysis that builds the data
> structures, etc, I assume?).
>
> I tried to clarify that in the class description -- does it help? Perhaps
> we should think of a better name, any ideas?
>

I'm not sure it'll be a big deal going forward - once there's a few uses I
guess most people will use it because they've seen an existing use (if
they've fixed some CFG analysis feature, they'll go looking for the
existing tests and find this, see its usage, etc)


>
> > How much more terse is this than the hand-written IR? (I assume that's
> the motivation/benefit?) How much does that help - perhaps you could show
> (not in anything that needs to be committed) an example of the textual IR
> for some of the tests, to demonstrate how much more terse it is? But I
> guess it's obvious enough perhaps from the tabular update array format that
> it is quite compact.
>
> Well, the thing is that writing the initial IR by hand is not that bad,
> but then you have to manually iterate over every basic block and assign it
> to a pointer (or a map of sort). The worst part is actually updating the IR
> within you test -- then you have to remember to update each terminator,
> handle edge cases when your successor is default switch case, change
> terminator to unreachable or return when you remove all the outgoing arcs,
> etc.
>
> As an example, you can look at the ToT DominatorTreeTest.cpp. The IR
> modified there is really small, yet the update code still manages to pretty
> lengthy and thus not particularly readable. If you are really interested, I
> can generate some real-world examples that directly compare the two
> described approaches.
>

Any chance that test cane be ported to this API? Might help demonstrate the
value, iron out use cases/features/etc. But I guess that test needs more
than just the CFG - add instructions, etc. Which aren't to be supported by
this API.

Which parts are not so readable? The code that initializes all the local
named variables to refer to the BBs & instructions? (YN and BBN)


>
> Thanks,
> ~Kuba
>
>
> https://reviews.llvm.org/D34798
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170629/034b2c03/attachment.html>