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

[Jakub Kuderski via Phabricator via llvm-commits]

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?

> 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.

Thanks,
~Kuba


https://reviews.llvm.org/D34798