Page MenuHomePhabricator

[gicombiner] Process the MatchDag such that every node is reachable from the roots
AcceptedPublic

Authored by dsanders on Oct 17 2019, 9:55 AM.

Details

Reviewers
volkan
bogner
Summary

When we build the walk across these DAG's we need to be able to reach every node
from the roots. Flip and traversal edges (so that use->def becomes def->uses)
that make nodes unreachable. Note that early on we'll just error out on these
flipped edges as def->uses edges are more complicated to match due to their
one->many nature.

Depends on D69077

Event Timeline

dsanders created this revision.Oct 17 2019, 9:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2019, 9:55 AM
Herald added a subscriber: dexonsmith. · View Herald Transcript
dsanders marked an inline comment as done.Oct 17 2019, 10:03 AM
dsanders added inline comments.
llvm/utils/TableGen/GICombinerEmitter.cpp
146

During an off-list review it was pointed out that this is quadratic in the worst case. I haven't fixed that yet but I don't think it's likely to be an issue in practice. This is partly due to the DAG's tending to be small (1-5 instr nodes) but is mostly due to the common case for the shape of the DAG.

The quadratic case is a 'straight-line' DAG of the form:

A->B->C->D->E

which can occasionally occur but is rarely more than 3 nodes long. The best case is a fan-out like:

A->{B,C,D,E}

which is linear but most DAGs will be roughly nlogn. For example:

A->{B,C}
B->{D,E}

If it becomes a bottleneck then I can improve this

volkan accepted this revision.Fri, Nov 15, 10:39 AM

LGTM with a couple of nits.

llvm/utils/TableGen/GICombinerEmitter.cpp
160

No need to create a new set for each edge, this can be moved outside of the loop.

189

Looks like there is no other debug output in this functions, is it worth keeping this?

This revision is now accepted and ready to land.Fri, Nov 15, 10:39 AM