This is an archive of the discontinued LLVM Phabricator instance.

-dot-machine-cfg for printing MachineFunction to a dot file
ClosedPublic

Authored by yassingh on Sep 12 2022, 10:07 AM.

Details

Summary

This pass allows a user to dump a MIR function to a dot file
and view it as a graph. It is targeted to provide a similar
functionality as -dot-cfg pass on LLVM-IR. As of now the pass
also support below flags:
-dot-mcfg-only [optional][won't print instructions in the
graph just block name]
-mcfg-dot-filename-prefix [optional][prefix to add to output dot file]
-mcfg-func-name [optional] [specify function name or it's
substring, handy if mir file contains multiple functions and
you need to see graph of just one]

More flags and details can be introduced as per the requirements
in future. This pass is inspired from -dot-cfg IR pass and APIs
are written in almost identical format.

Diff Detail

Event Timeline

yassingh created this revision.Sep 12 2022, 10:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 12 2022, 10:07 AM
yassingh requested review of this revision.Sep 12 2022, 10:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 12 2022, 10:07 AM
arsenm added inline comments.Sep 12 2022, 10:43 AM
llvm/include/llvm/CodeGen/MachineCFGPrinter.h
131

No else after return

llvm/lib/CodeGen/MachineCFGPrinter.cpp
3

Don't wrap this line

24–27

Could we reuse the same option names?

51

Single quote

llvm/test/Analysis/DotMachineCFG/AMDGPU/irreducible.mir
11–26

I don't see any point in checking the MIR output here. Check the dot output if anything

yassingh added inline comments.Sep 13 2022, 3:26 AM
llvm/lib/CodeGen/MachineCFGPrinter.cpp
24–27

Are you suggesting to use same option names as -dot-cfg?

llvm/test/Analysis/DotMachineCFG/AMDGPU/irreducible.mir
11–26

I was planning to add dot output test, accidently added this instead. Will submit the change with dot file test.

yassingh updated this revision to Diff 459797.Sep 13 2022, 10:28 AM

Changes addressing Matt's comments
Updated pass summary, code syntax and added tests

yassingh added inline comments.Sep 13 2022, 10:36 AM
llvm/lib/CodeGen/MachineCFGPrinter.cpp
24–27

I tried using the same cl option names, but it dosen't work.
"CommandLine Error: Option 'cfg-func-name' registered more than once! LLVM ERROR: inconsistency in registered CommandLine options "
I also referred /llvm/lib/Analysis/DDGPrinter.cpp which defines cl options for similar functionality eg "dot-ddg-filename-prefix" without reusing the same name.

arsenm added inline comments.Sep 13 2022, 12:41 PM
llvm/lib/CodeGen/MachineCFGPrinter.cpp
24–27

Yes, you would have to move the options into a common place, or have both printers in the same module which may not be worth the effort

cdevadas added inline comments.
llvm/include/llvm/CodeGen/MachineCFGPrinter.h
130

Leave a space between the return statements.

yassingh updated this revision to Diff 460771.Sep 16 2022, 8:21 AM
yassingh marked 3 inline comments as done.

Syntax changes addressing reviewers comments

yassingh edited the summary of this revision. (Show Details)Sep 16 2022, 8:22 AM
arsenm added inline comments.Sep 16 2022, 8:31 AM
llvm/include/llvm/CodeGen/MachineCFGPrinter.h
1

dropped c++ mode comment

Latest patch seems to have dropped the base patch

Latest patch seems to have dropped the base patch

Yes accidently overwrote the patch, updating patch with full diff

llvm/lib/CodeGen/MachineCFGPrinter.cpp
24–27

Yes, you would have to move the options into a common place, or have both printers in the same module which may not be worth the effort

yassingh updated this revision to Diff 460784.Sep 16 2022, 8:47 AM

Updating entire diff, since changes dropped the original patch, all syntax changes requested have been addressed.
Discussion about whether to move common cl-options for -dot-cfg and -dot-mcfg in single place is still open, but as @arsenm suggested it might not be worth the effort and I agree.

yassingh marked 3 inline comments as done.Sep 16 2022, 8:49 AM
arsenm added inline comments.Sep 16 2022, 8:52 AM
llvm/include/llvm/CodeGen/MachineCFGPrinter.h
90

Single quotes

98–123

This is duplicating the code in CFGPrinter. Can you templatify it to work on MachineBasicBlocks, similar to how the dominator tree does it?

yassingh updated this revision to Diff 461214.Sep 19 2022, 8:08 AM

Moved common code to generate node label to template functions

arsenm added inline comments.Sep 19 2022, 8:29 AM
llvm/include/llvm/Analysis/CFGPrinter.h
31

Doesn't match file naming convention. Also I would expect this to remain in something called CFGPrinter

yassingh updated this revision to Diff 461267.Sep 19 2022, 10:37 AM

Moved string label template functions back to CFGPrinter.h

arsenm accepted this revision.Sep 20 2022, 7:04 AM

LGTM with nits

llvm/include/llvm/Analysis/CFGPrinter.h
137

Don't need llvm::

139

Don't need llvm::

187

Don't need llvm::

llvm/include/llvm/CodeGen/MachineCFGPrinter.h
71

Don't need llvm::

This revision is now accepted and ready to land.Sep 20 2022, 7:04 AM
yassingh updated this revision to Diff 461917.Sep 21 2022, 9:11 AM

removed unnecessary namespace identifier llvm:: as @arsenm suggested

arsenm accepted this revision.Sep 21 2022, 11:58 AM
This revision was landed with ongoing or failed builds.Sep 22 2022, 12:19 AM
This revision was automatically updated to reflect the committed changes.