Page MenuHomePhabricator

[IDF] Teach Iterated Dominance Frontier to use a snapshot CFG based on a GraphDiff.

Authored by asbirlea on Aug 13 2018, 4:27 PM.



Create the ability to compute IDF using a CFG View.
For this, we'll need a new DT created using a list of Updates (to be refactored later to a GraphDiff), and the GraphTraits based on the same GraphDiff.

Diff Detail


Event Timeline

asbirlea created this revision.Aug 13 2018, 4:27 PM
kuhar added inline comments.Aug 13 2018, 4:33 PM
93 ↗(On Diff #160480)

Is it possible to instead create a helper function in GraffDiff that returns a node of appropriate type (like the pair here)?
Something like children(GraphDiff::make_node(GD, BB))

101 ↗(On Diff #160480)

I'd prefer braces around the for

Never touched this code, so all I can offer is a style nit. Thanks for the patch!

65 ↗(On Diff #160480)

nit: Unless this is consistent with the rest of the file, please spell this DoWork (there was a llvm-dev thread about this recently. My interpretation is that upper-camel is winning, but I'm also slightly biased.)

asbirlea added inline comments.Aug 13 2018, 10:46 PM
93 ↗(On Diff #160480)

I don't think that's how GraphTraits is intended to work, but your comment helped me understand that the InverseGraph bool is actually a property of the GraphDiff (just like doms), so it should be a template argument, not an argument to all methods.

I'll send a follow-up patch to the one adding GraphDiff to make that change and rebase this patch.
With that change, I can drop the if and this becomes:
children<std::pair<const GraphDiff<BasicBlock *, IsPostDom> *, NodeTy>>({GD, BB})
which I think is pretty clean.

asbirlea updated this revision to Diff 160515.Aug 13 2018, 10:59 PM

Address style comments.
Rebase on top of patch making GraphDiff templated on IsPostDom/InverseGraph.

asbirlea updated this revision to Diff 160516.Aug 13 2018, 11:03 PM
asbirlea marked 4 inline comments as done.


kuhar accepted this revision.Aug 17 2018, 10:11 AM

Seems fine

This revision is now accepted and ready to land.Aug 17 2018, 10:11 AM
This revision was automatically updated to reflect the committed changes.