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

[yassingh]

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.

[arsenm]

llvm/include/llvm/CodeGen/MachineCFGPrinter.h

130

No else after return

llvm/lib/CodeGen/MachineCFGPrinter.cpp

2

Don't wrap this line

23–26

Could we reuse the same option names?

50

Single quote

llvm/test/Analysis/DotMachineCFG/AMDGPU/irreducible.mir

10–25

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

[yassingh]

llvm/lib/CodeGen/MachineCFGPrinter.cpp

23–26

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

llvm/test/Analysis/DotMachineCFG/AMDGPU/irreducible.mir

10–25

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

[yassingh]

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

[yassingh]

llvm/lib/CodeGen/MachineCFGPrinter.cpp

23–26

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]

llvm/lib/CodeGen/MachineCFGPrinter.cpp

23–26

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]

llvm/include/llvm/CodeGen/MachineCFGPrinter.h

130

Leave a space between the return statements.

[yassingh]

Syntax changes addressing reviewers comments

[arsenm]

llvm/include/llvm/CodeGen/MachineCFGPrinter.h

1

dropped c++ mode comment

[arsenm]

Latest patch seems to have dropped the base patch

[yassingh]

In D133709#3795640, @arsenm wrote:

Latest patch seems to have dropped the base patch

Yes accidently overwrote the patch, updating patch with full diff

llvm/lib/CodeGen/MachineCFGPrinter.cpp

23–26

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]

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.

[arsenm]

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]

Moved common code to generate node label to template functions

[arsenm]

llvm/include/llvm/Analysis/CFGPrinter.h

31 ↗

(On Diff #461214)

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

[yassingh]

Moved string label template functions back to CFGPrinter.h

[arsenm]

LGTM with nits

llvm/include/llvm/Analysis/CFGPrinter.h

137 ↗

(On Diff #461267)

Don't need llvm::

139 ↗

(On Diff #461267)

Don't need llvm::

187 ↗

(On Diff #461267)

Don't need llvm::

llvm/include/llvm/CodeGen/MachineCFGPrinter.h

71

Don't need llvm::

[yassingh]

removed unnecessary namespace identifier llvm:: as @arsenm suggested