Page MenuHomePhabricator

Utility to dump .dot representation of SelectionDAG without firing viewer
ClosedPublic

Authored by madhur13490 on May 28 2020, 4:05 AM.

Details

Summary

This patch adds support for dumping .dot
representation of SelectionDAG. It is inspired from the fact that,
a developer may want to just dump the graph at
a predictable path with a simple name to compare.
The exisitng utility (i.e. viewGraph) are overkill
for this motive hence this patch adds the requires support
while using the core routines from GraphWriter.

Example usage: DAG.dumpDotGraph("/tmp/graph.dot", "MyGraph")
will create /tmp/graph.dot file when DAG is an
object of SelectionDAG class.

Diff Detail

Event Timeline

madhur13490 created this revision.May 28 2020, 4:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 28 2020, 4:05 AM

Update summary

madhur13490 edited the summary of this revision. (Show Details)May 28 2020, 9:43 AM
arsenm added inline comments.May 28 2020, 10:17 AM
llvm/include/llvm/CodeGen/SelectionDAG.h
443

I would expect the filename to be a StringRef (I guess debuggers struggle more with this, and the others are using std::string)

llvm/include/llvm/Support/GraphWriter.h
344–345

The existing done I think was sufficient

366–369

Don't see the point of getting the return string and check if it's empty?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
171

Should this fully remove the code depending on LLVM_ENABLE_DUMP? Also the declaration should get LLVM_DUMP_METHOD?

madhur13490 marked 5 inline comments as done.May 29 2020, 4:08 AM
madhur13490 added inline comments.
llvm/include/llvm/CodeGen/SelectionDAG.h
443

In fact, I am preferring Twine here because I may want to concat input function name, code function name and some state. e.g filename = MachineFunction.getName() + "LowerCall"+"start" to indicate DAG for machine function in LowerCall at the start!

llvm/include/llvm/Support/GraphWriter.h
344–345

No, just "done" doesn't indicate who's and what's done? There has to be some context to it and it is present with view* methods. So, I'd prefer emitting more verbose text.

366–369

Yeah. Removed.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
171

Sure, the method will be more accessible in that way.

madhur13490 marked an inline comment as done.
madhur13490 edited the summary of this revision. (Show Details)

Addressing comments by arsenm

Fix clang-tidy errors

arsenm added inline comments.May 29 2020, 12:58 PM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
178–179

I think having anything here defeats the point of having a compile time way to remove the function. The function should not be present in the build at all without LLVM_ENABLE_DUMP

Address further comment by arsenm

madhur13490 marked an inline comment as done.May 30 2020, 9:26 PM
madhur13490 added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGPrinter.cpp
178–179

Yes, done.

arsenm accepted this revision.Jun 2 2020, 6:46 AM

LGTM with nit

llvm/include/llvm/Support/GraphWriter.h
345

Remove the word "the"

This revision is now accepted and ready to land.Jun 2 2020, 6:46 AM
This revision was automatically updated to reflect the committed changes.