This is an archive of the discontinued LLVM Phabricator instance.

Differential D106253

[mlir] Improve Graphviz visualization in PrintOpPass
ClosedPublic

Authored by springerm on Jul 18 2021, 11:50 PM.

Details

Reviewers
rriddle
nicolasvasilache
ftynse
Commits
rG8d15b7dcbaa1: [mlir] Improve Graphviz visualization in PrintOpPass
Summary
  • Visualize blocks and regions as subgraphs.
  • Generate DOT file directly instead of using GraphTraits. GraphTraits does not support subgraphs, colors, etc.

Diff Detail

Event Timeline

springerm created this revision.Jul 18 2021, 11:50 PM
Herald added subscribers: dcaballe, cota, teijeong and 16 others. · View Herald TranscriptJul 18 2021, 11:50 PM
springerm requested review of this revision.Jul 18 2021, 11:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2021, 11:50 PM
Herald added subscribers: stephenneuendorffer, nicolasvasilache. · View Herald Transcript
springerm added reviewers: rriddle, nicolasvasilache, ftynse.Jul 18 2021, 11:51 PM
rriddle requested changes to this revision.Jul 18 2021, 11:55 PM
rriddle added inline comments.
mlir/include/mlir/Transforms/Passes.td
703–704

Can you split these features into separate commits? Some of them have less obvious value than others. It also makes the review easier.

mlir/lib/Transforms/ViewOpGraph.cpp
18

Namespaces should only really contain classes. Please move everything else out.

93–94

Please use proper C++ constructor field initialization.

262

The llvm:: here shouldn't be necessary.

This revision now requires changes to proceed.Jul 18 2021, 11:55 PM
Harbormaster completed remote builds in B114777: Diff 359675.Jul 19 2021, 12:20 AM
springerm updated this revision to Diff 359747.Jul 19 2021, 4:40 AM
springerm marked an inline comment as done.

address comments

springerm marked 3 inline comments as done.Jul 19 2021, 4:41 AM
springerm updated this revision to Diff 359748.Jul 19 2021, 4:43 AM

remove unnecessary line

springerm added a comment.Jul 19 2021, 4:57 AM

For reference:

springerm updated this revision to Diff 359752.Jul 19 2021, 5:02 AM

simplify code

Harbormaster completed remote builds in B114833: Diff 359752.Jul 19 2021, 5:34 AM
jpienaar added a subscriber: jpienaar.Jul 19 2021, 6:44 AM
jpienaar added inline comments.
mlir/lib/Transforms/ViewOpGraph.cpp
19

I'd rather have this be kShapeNode which documents it's use rather than have kX == lowercase(X), else seems like a magical constant with extra step.

66

There can be multiple visualizations of an MLIR module (dominated by, will execute after, CDG, ...) it would be good to spell this out what this particular one is visualizing (which could refer to the description of pass for more info if appropriate)

83

We have an indented ostream, why not use that?

180

We have operations with very large number of results, always adding them to label will make this unusable there.

194

When I did one of these recently I made the arguments a table, that way arguments were just named numerically and associated with block entry. It kept all the arguments together and avoided it appearing the same as ops.

springerm marked an inline comment as done.Jul 19 2021, 6:56 PM
springerm added inline comments.
mlir/lib/Transforms/ViewOpGraph.cpp
66

Done. This is a dataflow visualization.

Btw, I'm planning to add a CFG visualization (to be enabled via a pass option) in a subsequent commit.

83

Yes, raw_indented_ostream is what we need here. But I couldn't find a way to use it in a pass.

The problem is that:

  • raw_indented_ostream's copy constructor is deleted.
  • The generated clonePass function in mlir/Transforms/Passes.h.inc calls the copy constructor of the pass.
  • Therefore, the raw_indented_ostream would have to be passed in as a reference. But there's no other suitable place to create a raw_indented_ostream (other than the constructor of the pass).
  • Alternative: Change tools/mlir-tblgen/PassGen.cpp and add a tablegen option to avoid emitting clonePass functions.
180

I'd add this in a separate commit if that's OK. (To keep this one small.) To be enabled via a pass option.

194

Not sure what you mean by that. Are you referring to Graphviz HTML tables? (https://graphviz.org/doc/info/shapes.html#html) With Graphviz tables I could generate a single node for all block arguments of a block. Then draw arrows from each cell to their respective uses.

Or do you recommend not generating nodes for block arguments at all?

Btw, in some of my experiments I found it useful to duplicate block argument nodes and constant nodes (e.g. %c0). Such nodes are often used many times throughout the IR, which renders quite unreadable due to the many edges. Also, those edges are quite long. What helps is creating a new block arg/constant node for each use, which the Graphviz engine will render close to the user, resulting in shorter edges (and less lines on the screen). I'll add this functionality in a subsequent commit.

springerm updated this revision to Diff 359984.Jul 19 2021, 6:57 PM

address comments

Harbormaster completed remote builds in B114999: Diff 359984.Jul 19 2021, 7:33 PM
springerm added a child revision: D106337: [mlir] Truncate/skip long strings in ViewOpGraph.cpp.Jul 19 2021, 8:09 PM
springerm updated this revision to Diff 360007.Jul 19 2021, 9:30 PM

minor improvements

Harbormaster completed remote builds in B115016: Diff 360007.Jul 19 2021, 10:10 PM
jpienaar added a comment.Jul 22 2021, 7:41 AM
In D106253#2887020, @springerm wrote:

For reference:

The floating 0 below test.br is weird, could that be avoided?

mlir/lib/Transforms/ViewOpGraph.cpp
83

Yes, raw_ostream's copy constructor is deleted, which results in raw_indented_ostream's being deleted. If you just added a copy constructor for this pass where you create a new one, that should work for this case (what would happen I believe is a it would be created with underlying raw_ostream, so you'd lose the indendation of the passed in indented ostream [which is fine as there is no expectation that the cloned pass and the original share indentation]).

113

Why not combine these? Use interleaveComma on the map & in functor create the string rather than creating all the strings and then outputting.

194

Yes, an HTML table. But indeed I ran into the same issue with long lines on larger graphs. Duplication is good idea for keeping it scoped - it makes it more difficult to notice when 2 ops have the same input (so if one has patterns that match equal operands then that is more desirable).

springerm marked 2 inline comments as done.Jul 22 2021, 11:02 PM
In D106253#2896443, @jpienaar wrote:
In D106253#2887020, @springerm wrote:

For reference:

The floating 0 below test.br is weird, could that be avoided?

This is a rendering issue. Edges have labels to indicate that a value is used as the 0th, 1st, 2nd, etc. operand. Those edges are clipped at cluster (subgraph) boundaries, but for some reason the labels are not. I think this is a bug in Graphviz. As a workaround, I stopped printing labels for edges that start/end at a cluster boundary.

springerm updated this revision to Diff 361089.Jul 22 2021, 11:03 PM

address comments

Harbormaster completed remote builds in B115777: Diff 361089.Jul 22 2021, 11:14 PM
springerm updated this revision to Diff 361435.Jul 24 2021, 4:39 AM

minor update

Harbormaster completed remote builds in B116019: Diff 361435.Jul 24 2021, 4:53 AM
springerm added a comment.Jul 29 2021, 8:36 PM

Are there any other changes that I should make to this revision?

In case you are worried about the std::string, which are considered a code smell as you said: There is a way to avoid a few of the std::string. By writing writing Graphviz attributes directly onto the stream instead of storing them in an AttributeMap. But that would reduce code readability and some level of abstraction.

Basically, I could no longer use helper functions such as emitAttrList. These would have to be duplicated wherever attributes are printed. Also more lambdas are likely needed for printing labels. The main reason why I added emitAttrList and AttributeMap is that it gives the code some more structure. Note that parts of the function names are derived from the DOT language specification. E.g., the language spec defines things such as attr_list. I tried to mirror that structure in the code as good as possible.

For reference: DOT language spec is here: https://graphviz.org/doc/info/lang.html

Herald added a subscriber: Chia-hungDuan. · View Herald TranscriptJul 29 2021, 8:36 PM
rriddle accepted this revision.Aug 3 2021, 6:01 PM

Sorry, heavily in and out of OOO recently.

Took a glance, looks good to land after resolving comments. Thanks!

mlir/include/mlir/Transforms/Passes.td
690–691

Unrelated, but I don't think this should be a ModuleOp pass. This feels like it should be a general "any op" pass.

mlir/lib/Transforms/ViewOpGraph.cpp
49–50

Can you move this to the description field of the pass?

59–61
64

Optional is exported in the mlir namespace, so the llvm:: here can be dropped.

85
195
203
257
263

Where is plainOs used? It looks like it can be dropped.

This revision is now accepted and ready to land.Aug 3 2021, 6:01 PM
Herald added a subscriber: wrengr. · View Herald TranscriptAug 3 2021, 6:01 PM
springerm marked 7 inline comments as done.Aug 3 2021, 6:58 PM

Thanks for the review!

mlir/lib/Transforms/ViewOpGraph.cpp
263

This is necessary to implement the copy constructor (required by clonePass) correctly. There's no way to copy the indented output stream, so I have to construct a new one. That's where I pass in plainOs.

See my response to jpienaar's comment regarding the indented output stream for more details.

springerm updated this revision to Diff 363934.Aug 3 2021, 7:00 PM

address comments

rriddle added inline comments.Aug 3 2021, 7:06 PM
mlir/lib/Transforms/ViewOpGraph.cpp
263

Can we just add a raw_ostream &getOStream() { return os; } method to raw_indented_ostream then? The use of plainOs feels a bit hacky.

Harbormaster completed remote builds in B117790: Diff 363934.Aug 3 2021, 7:11 PM
springerm updated this revision to Diff 363938.Aug 3 2021, 7:12 PM

no change

Harbormaster completed remote builds in B117794: Diff 363938.Aug 3 2021, 7:23 PM
springerm updated this revision to Diff 363941.Aug 3 2021, 7:24 PM
springerm marked an inline comment as done.

address comments

springerm edited the summary of this revision. (Show Details)Aug 3 2021, 7:25 PM
Harbormaster completed remote builds in B117797: Diff 363941.Aug 3 2021, 7:34 PM
This revision was landed with ongoing or failed builds.Aug 3 2021, 7:57 PM
Closed by commit rG8d15b7dcbaa1: [mlir] Improve Graphviz visualization in PrintOpPass (authored by springerm). · Explain Why
This revision was automatically updated to reflect the committed changes.
springerm added a commit: rG8d15b7dcbaa1: [mlir] Improve Graphviz visualization in PrintOpPass.
Herald added a subscriber: mgorny. · View Herald TranscriptAug 3 2021, 7:57 PM
springerm mentioned this in D107424: [mlir] Fix broken build in pass_manager.py.Aug 3 2021, 9:11 PM
springerm mentioned this in rGfaeb7ec68bc1: [mlir] Fix broken build in pass_manager.py.Aug 3 2021, 9:31 PM

Revision Contents

PathSize
mlir/
include/
mlir/
Support/
3 lines
Transforms/
18 lines
19 lines
lib/
Transforms/
1 line
314 lines
test/
Transforms/
32 lines

Diff 363948

mlir/include/mlir/Support/IndentedOstream.h

Show All 39 Lines struct DelimitedScope {
} }
raw_indented_ostream &os; raw_indented_ostream &os;
private: private:
llvm::StringRef open, close; llvm::StringRef open, close;
}; };
/// Returns the underlying (unindented) raw_ostream.
raw_ostream &getOStream() const { return os; }
/// Returns DelimitedScope. /// Returns DelimitedScope.
DelimitedScope scope(StringRef open = "", StringRef close = "") { DelimitedScope scope(StringRef open = "", StringRef close = "") {
return DelimitedScope(*this, open, close); return DelimitedScope(*this, open, close);
} }
/// Re-indents by removing the leading whitespace from the first non-empty /// Re-indents by removing the leading whitespace from the first non-empty
/// line from every line of the string, skipping over empty lines at the /// line from every line of the string, skipping over empty lines at the
/// start. /// start.
▲ Show 20 Lines • Show All 47 Lines • Show Last 20 Lines

mlir/include/mlir/Transforms/Passes.td

Show First 20 Lines • Show All 681 Lines • ▼ Show 20 Lines let description = [{
} }
``` ```
See [Symbols and SymbolTables](SymbolsAndSymbolTables.md) for more See [Symbols and SymbolTables](SymbolsAndSymbolTables.md) for more
information on `Symbols`. information on `Symbols`.
}]; }];
let constructor = "mlir::createSymbolDCEPass()"; let constructor = "mlir::createSymbolDCEPass()";
} }
def ViewOpGraphPass : Pass<"view-op-graph", "ModuleOp"> { def ViewOpGraphPass : Pass<"view-op-graph"> {
rriddleUnsubmitted
Done
ReplyInline Actions

Unrelated, but I don't think this should be a ModuleOp pass. This feels like it should be a general "any op" pass.

rriddle: Unrelated, but I don't think this should be a ModuleOp pass. This feels like it should be a…
let summary = "Print graphviz view of module"; let summary = "Print Graphviz dataflow visualization of an operation";
let description = [{ let description = [{
This pass prints a graphviz per block of a module. This pass prints a Graphviz dataflow graph of a module.
- Op are represented as nodes; - Operations are represented as nodes;
- Uses as edges; - Uses as edges;
- Regions/blocks as subgraphs.
Note: See https://www.graphviz.org/doc/info/lang.html for more information
about the Graphviz DOT language.
}]; }];
let constructor = "mlir::createPrintOpGraphPass()"; let constructor = "mlir::createPrintOpGraphPass()";
let options = [
Option<"title", "title", "std::string",
/*default=*/"", "The prefix of the title of the graph">,
Option<"shortNames", "short-names", "bool", /*default=*/"false",
"Use short names">
];
} }
rriddleUnsubmitted
Done
ReplyInline Actions

Can you split these features into separate commits? Some of them have less obvious value than others. It also makes the review easier.

rriddle: Can you split these features into separate commits? Some of them have less obvious value than…
#endif // MLIR_TRANSFORMS_PASSES #endif // MLIR_TRANSFORMS_PASSES

mlir/include/mlir/Transforms/ViewOpGraph.h

//===- ViewOpGraph.h - View/write op graphviz graphs ------------*- C++ -*-===// //===- ViewOpGraph.h - View/write op graphviz graphs ------------*- C++ -*-===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Defines interface to produce Graphviz outputs of MLIR op within block. // Defines interface to produce Graphviz outputs of MLIR op within block.
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef MLIR_TRANSFORMS_VIEWOPGRAPH_H_ #ifndef MLIR_TRANSFORMS_VIEWOPGRAPH_H_
#define MLIR_TRANSFORMS_VIEWOPGRAPH_H_ #define MLIR_TRANSFORMS_VIEWOPGRAPH_H_
#include "mlir/Support/LLVM.h" #include "mlir/Support/LLVM.h"
#include "llvm/Support/GraphWriter.h"
#include "llvm/Support/raw_ostream.h" #include "llvm/Support/raw_ostream.h"
namespace mlir { namespace mlir {
class Block; class Pass;
class ModuleOp;
template <typename T> class OperationPass;
/// Displays the graph in a window. This is for use from the debugger and
/// depends on Graphviz to generate the graph.
void viewGraph(Block &block, const Twine &name, bool shortNames = false,
const Twine &title = "",
llvm::GraphProgram::Name program = llvm::GraphProgram::DOT);
raw_ostream &writeGraph(raw_ostream &os, Block &block, bool shortNames = false,
const Twine &title = "");
/// Creates a pass to print op graphs. /// Creates a pass to print op graphs.
std::unique_ptr<OperationPass<ModuleOp>> std::unique_ptr<Pass>
createPrintOpGraphPass(raw_ostream &os = llvm::errs(), bool shortNames = false, createPrintOpGraphPass(raw_ostream &os = llvm::errs());
const Twine &title = "");
} // end namespace mlir } // end namespace mlir
#endif // MLIR_TRANSFORMS_VIEWOPGRAPH_H_ #endif // MLIR_TRANSFORMS_VIEWOPGRAPH_H_

mlir/lib/Transforms/CMakeLists.txt

Show All 33 Linesadd_mlir_library(MLIRTransforms
MLIRAffine MLIRAffine
MLIRAnalysis MLIRAnalysis
MLIRCopyOpInterface MLIRCopyOpInterface
MLIRLinalg MLIRLinalg
MLIRLoopLikeInterface MLIRLoopLikeInterface
MLIRMemRef MLIRMemRef
MLIRSCF MLIRSCF
MLIRPass MLIRPass
MLIRSupportIndentedOstream
MLIRTransformUtils MLIRTransformUtils
MLIRVector MLIRVector
) )

mlir/lib/Transforms/ViewOpGraph.cpp

//===- ViewOpGraph.cpp - View/write op graphviz graphs --------------------===// //===- ViewOpGraph.cpp - View/write op graphviz graphs --------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#include "mlir/Transforms/ViewOpGraph.h" #include "mlir/Transforms/ViewOpGraph.h"
#include "PassDetail.h" #include "PassDetail.h"
#include "mlir/IR/Block.h" #include "mlir/IR/Block.h"
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/Operation.h" #include "mlir/IR/Operation.h"
#include "llvm/Support/CommandLine.h" #include "mlir/Support/IndentedOstream.h"
#include "llvm/Support/Format.h"
using namespace mlir; using namespace mlir;
static const StringRef kLineStyleDataFlow = "solid";
rriddleUnsubmitted
Done
ReplyInline Actions

Namespaces should only really contain classes. Please move everything else out.

rriddle: Namespaces should only really contain classes. Please move everything else out.
static const StringRef kShapeNode = "ellipse";
jpienaarUnsubmitted
Done
ReplyInline Actions

I'd rather have this be kShapeNode which documents it's use rather than have kX == lowercase(X), else seems like a magical constant with extra step.

jpienaar: I'd rather have this be kShapeNode which documents it's use rather than have kX == lowercase(X)…
static const StringRef kShapeNone = "plain";
/// Return the size limits for eliding large attributes. /// Return the size limits for eliding large attributes.
static int64_t getLargeAttributeSizeLimit() { static int64_t getLargeAttributeSizeLimit() {
// Use the default from the printer flags if possible. // Use the default from the printer flags if possible.
if (Optional<int64_t> limit = OpPrintingFlags().getLargeElementsAttrLimit()) if (Optional<int64_t> limit = OpPrintingFlags().getLargeElementsAttrLimit())
return *limit; return *limit;
return 16; return 16;
} }
namespace llvm { /// Return all values printed onto a stream as a string.
static std::string strFromOs(function_ref<void(raw_ostream &)> func) {
// Specialize GraphTraits to treat Block as a graph of Operations as nodes and std::string buf;
// uses as edges. llvm::raw_string_ostream os(buf);
template <> struct GraphTraits<Block *> { func(os);
using GraphType = Block *; return os.str();
using NodeRef = Operation *;
using ChildIteratorType = Operation::user_iterator;
static ChildIteratorType child_begin(NodeRef n) { return n->user_begin(); }
static ChildIteratorType child_end(NodeRef n) { return n->user_end(); }
// Operation's destructor is private so use Operation* instead and use
// mapped iterator.
static Operation *AddressOf(Operation &op) { return &op; }
using nodes_iterator = mapped_iterator<Block::iterator, decltype(&AddressOf)>;
static nodes_iterator nodes_begin(Block *b) {
return nodes_iterator(b->begin(), &AddressOf);
} }
static nodes_iterator nodes_end(Block *b) {
return nodes_iterator(b->end(), &AddressOf); /// Escape special characters such as '\n' and quotation marks.
static std::string escapeString(std::string str) {
return strFromOs([&](raw_ostream &os) { os.write_escaped(str); });
} }
};
// Specialize DOTGraphTraits to produce more readable output. /// Put quotation marks around a given string.
template <> struct DOTGraphTraits<Block *> : public DefaultDOTGraphTraits { static std::string quoteString(std::string str) { return "\"" + str + "\""; }
using DefaultDOTGraphTraits::DefaultDOTGraphTraits;
static std::string getNodeLabel(Operation *op, Block *); using AttributeMap = llvm::StringMap<std::string>;
namespace {
/// This struct represents a node in the DOT language. Each node has an
rriddleUnsubmitted
Done
ReplyInline Actions

Can you move this to the description field of the pass?

rriddle: Can you move this to the description field of the pass?
/// identifier and an optional identifier for the cluster (subgraph) that
/// contains the node.
/// Note: In the DOT language, edges can be drawn only from nodes to nodes, but
/// not between clusters. However, edges can be clipped to the boundary of a
/// cluster with `lhead` and `ltail` attributes. Therefore, when creating a new
/// cluster, an invisible "anchor" node is created.
struct Node {
public:
Node(int id = 0, Optional<int> clusterId = llvm::None)
: id(id), clusterId(clusterId) {}
rriddleUnsubmitted
Done
ReplyInline Actions
public:
- Node() {}
- Node(int id) : id(id), clusterId(llvm::None) {}
- Node(int id, Optional<int> clusterId) : id(id), clusterId(clusterId) {}
+ Node(int id = 0, Optional<int> clusterId = llvm::None) : id(id), clusterId(clusterId) {}
int id;
rriddle:
int id;
Optional<int> clusterId;
}; };
rriddleUnsubmitted
Done
ReplyInline Actions

Optional is exported in the mlir namespace, so the llvm:: here can be dropped.

rriddle: Optional is exported in the mlir namespace, so the llvm:: here can be dropped.
std::string DOTGraphTraits<Block *>::getNodeLabel(Operation *op, Block *b) { /// This pass generates a Graphviz dataflow visualization of an MLIR operation.
jpienaarUnsubmitted
Not Done
ReplyInline Actions

There can be multiple visualizations of an MLIR module (dominated by, will execute after, CDG, ...) it would be good to spell this out what this particular one is visualizing (which could refer to the description of pass for more info if appropriate)

jpienaar: There can be multiple visualizations of an MLIR module (dominated by, will execute after, CDG, .
springermAuthorUnsubmitted
Done
ReplyInline Actions

Done. This is a dataflow visualization.

Btw, I'm planning to add a CFG visualization (to be enabled via a pass option) in a subsequent commit.

springerm: Done. This is a dataflow visualization. Btw, I'm planning to add a CFG visualization (to be…
// Reuse the print output for the node labels. /// Note: See https://www.graphviz.org/doc/info/lang.html for more information
std::string ostr; /// about the Graphviz DOT language.
raw_string_ostream os(ostr); class PrintOpPass : public ViewOpGraphPassBase<PrintOpPass> {
os << op->getName() << "\n"; public:
PrintOpPass(raw_ostream &os) : os(os) {}
if (!op->getLoc().isa<UnknownLoc>()) { PrintOpPass(const PrintOpPass &o) : os(o.os.getOStream()) {}
os << op->getLoc() << "\n";
void runOnOperation() override {
emitGraph([&]() {
processOperation(getOperation());
emitAllEdgeStmts();
});
} }
// Print resultant types private:
llvm::interleaveComma(op->getResultTypes(), os); /// Emit all edges. This function should be called after all nodes have been
os << "\n"; /// emitted.
jpienaarUnsubmitted
Not Done
ReplyInline Actions

We have an indented ostream, why not use that?

jpienaar: We have an indented ostream, why not use that?
springermAuthorUnsubmitted
Done
ReplyInline Actions

Yes, raw_indented_ostream is what we need here. But I couldn't find a way to use it in a pass.

The problem is that:

  • raw_indented_ostream's copy constructor is deleted.
  • The generated clonePass function in mlir/Transforms/Passes.h.inc calls the copy constructor of the pass.
  • Therefore, the raw_indented_ostream would have to be passed in as a reference. But there's no other suitable place to create a raw_indented_ostream (other than the constructor of the pass).
  • Alternative: Change tools/mlir-tblgen/PassGen.cpp and add a tablegen option to avoid emitting clonePass functions.
springerm: Yes, `raw_indented_ostream` is what we need here. But I couldn't find a way to use it in a pass.
jpienaarUnsubmitted
Done
ReplyInline Actions

Yes, raw_ostream's copy constructor is deleted, which results in raw_indented_ostream's being deleted. If you just added a copy constructor for this pass where you create a new one, that should work for this case (what would happen I believe is a it would be created with underlying raw_ostream, so you'd lose the indendation of the passed in indented ostream [which is fine as there is no expectation that the cloned pass and the original share indentation]).

jpienaar: Yes, raw_ostream's copy constructor is deleted, which results in raw_indented_ostream's being…
void emitAllEdgeStmts() {
for (const std::string &edge : edges)
rriddleUnsubmitted
Done
ReplyInline Actions
void emitAllEdgeStmts() {
- for (const auto &edge : edges)
+ for (StringRef edge : edges)
os << edge << ";\n";
rriddle:
os << edge << ";\n";
edges.clear();
}
/// Emit a cluster (subgraph). The specified builder generates the body of the
/// cluster. Return the anchor node of the cluster.
Node emitClusterStmt(function_ref<void()> builder, std::string label = "") {
int clusterId = ++counter;
os << "subgraph cluster_" << clusterId << " {\n";
rriddleUnsubmitted
Done
ReplyInline Actions

Please use proper C++ constructor field initialization.

rriddle: Please use proper C++ constructor field initialization.
os.indent();
// Emit invisible anchor node from/to which arrows can be drawn.
Node anchorNode = emitNodeStmt(" ", kShapeNone);
os << attrStmt("label", quoteString(escapeString(label))) << ";\n";
builder();
os.unindent();
os << "}\n";
return Node(anchorNode.id, clusterId);
}
/// Generate an attribute statement.
std::string attrStmt(const Twine &key, const Twine &value) {
return (key + " = " + value).str();
}
/// Emit an attribute list.
void emitAttrList(raw_ostream &os, const AttributeMap &map) {
os << "[";
interleaveComma(map, os, [&](const auto &it) {
jpienaarUnsubmitted
Done
ReplyInline Actions

Why not combine these? Use interleaveComma on the map & in functor create the string rather than creating all the strings and then outputting.

jpienaar: Why not combine these? Use interleaveComma on the map & in functor create the string rather…
os << attrStmt(it.getKey(), it.getValue());
});
os << "]";
}
// Print an MLIR attribute to `os`. Large attributes are truncated.
void emitMlirAttr(raw_ostream &os, Attribute attr) {
// A value used to elide large container attribute. // A value used to elide large container attribute.
int64_t largeAttrLimit = getLargeAttributeSizeLimit(); int64_t largeAttrLimit = getLargeAttributeSizeLimit();
for (auto attr : op->getAttrs()) {
os << '\n' << attr.first << ": ";
// Always emit splat attributes. // Always emit splat attributes.
if (attr.second.isa<SplatElementsAttr>()) { if (attr.isa<SplatElementsAttr>()) {
attr.second.print(os); attr.print(os);
continue; return;
} }
// Elide "big" elements attributes. // Elide "big" elements attributes.
auto elements = attr.second.dyn_cast<ElementsAttr>(); auto elements = attr.dyn_cast<ElementsAttr>();
if (elements && elements.getNumElements() > largeAttrLimit) { if (elements && elements.getNumElements() > largeAttrLimit) {
os << std::string(elements.getType().getRank(), '[') << "..." os << std::string(elements.getType().getRank(), '[') << "..."
<< std::string(elements.getType().getRank(), ']') << " : " << std::string(elements.getType().getRank(), ']') << " : "
<< elements.getType(); << elements.getType();
continue; return;
} }
auto array = attr.second.dyn_cast<ArrayAttr>(); auto array = attr.dyn_cast<ArrayAttr>();
if (array && static_cast<int64_t>(array.size()) > largeAttrLimit) { if (array && static_cast<int64_t>(array.size()) > largeAttrLimit) {
os << "[...]"; os << "[...]";
continue; return;
} }
// Print all other attributes. // Print all other attributes.
attr.second.print(os); attr.print(os);
}
return os.str();
}
} // end namespace llvm
namespace {
// PrintOpPass is simple pass to write graph per function.
// Note: this is a module pass only to avoid interleaving on the same ostream
// due to multi-threading over functions.
class PrintOpPass : public ViewOpGraphPassBase<PrintOpPass> {
public:
PrintOpPass(raw_ostream &os, bool shortNames, const Twine &title) : os(os) {
this->shortNames = shortNames;
this->title = title.str();
} }
std::string getOpName(Operation &op) { /// Append an edge to the list of edges.
auto symbolAttr = /// Note: Edges are written to the output stream via `emitAllEdgeStmts`.
op.getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName()); void emitEdgeStmt(Node n1, Node n2, std::string label,
if (symbolAttr) StringRef style = kLineStyleDataFlow) {
return std::string(symbolAttr.getValue()); AttributeMap attrs;
++unnamedOpCtr; attrs["style"] = style.str();
return (op.getName().getStringRef() + llvm::utostr(unnamedOpCtr)).str(); // Do not label edges that start/end at a cluster boundary. Such edges are
} // clipped at the boundary, but labels are not. This can lead to labels
// floating around without any edge next to them.
if (!n1.clusterId && !n2.clusterId)
attrs["label"] = quoteString(escapeString(label));
// Use `ltail` and `lhead` to draw edges between clusters.
if (n1.clusterId)
attrs["ltail"] = "cluster_" + std::to_string(*n1.clusterId);
if (n2.clusterId)
attrs["lhead"] = "cluster_" + std::to_string(*n2.clusterId);
edges.push_back(strFromOs([&](raw_ostream &os) {
os << llvm::format("v%i -> v%i ", n1.id, n2.id);
emitAttrList(os, attrs);
}));
}
/// Emit a graph. The specified builder generates the body of the graph.
void emitGraph(function_ref<void()> builder) {
os << "digraph G {\n";
os.indent();
// Edges between clusters are allowed only in compound mode.
os << attrStmt("compound", "true") << ";\n";
builder();
os.unindent();
os << "}\n";
jpienaarUnsubmitted
Not Done
ReplyInline Actions

We have operations with very large number of results, always adding them to label will make this unusable there.

jpienaar: We have operations with very large number of results, always adding them to label will make…
springermAuthorUnsubmitted
Done
ReplyInline Actions

I'd add this in a separate commit if that's OK. (To keep this one small.) To be enabled via a pass option.

springerm: I'd add this in a separate commit if that's OK. (To keep this one small.) To be enabled via a…
}
/// Emit a node statement.
Node emitNodeStmt(std::string label, StringRef shape = kShapeNode) {
int nodeId = ++counter;
AttributeMap attrs;
attrs["label"] = quoteString(escapeString(label));
attrs["shape"] = shape.str();
os << llvm::format("v%i ", nodeId);
emitAttrList(os, attrs);
os << ";\n";
return Node(nodeId);
}
jpienaarUnsubmitted
Not Done
ReplyInline Actions

When I did one of these recently I made the arguments a table, that way arguments were just named numerically and associated with block entry. It kept all the arguments together and avoided it appearing the same as ops.

jpienaar: When I did one of these recently I made the arguments a table, that way arguments were just…
springermAuthorUnsubmitted
Done
ReplyInline Actions

Not sure what you mean by that. Are you referring to Graphviz HTML tables? (https://graphviz.org/doc/info/shapes.html#html) With Graphviz tables I could generate a single node for all block arguments of a block. Then draw arrows from each cell to their respective uses.

Or do you recommend not generating nodes for block arguments at all?

Btw, in some of my experiments I found it useful to duplicate block argument nodes and constant nodes (e.g. %c0). Such nodes are often used many times throughout the IR, which renders quite unreadable due to the many edges. Also, those edges are quite long. What helps is creating a new block arg/constant node for each use, which the Graphviz engine will render close to the user, resulting in shorter edges (and less lines on the screen). I'll add this functionality in a subsequent commit.

springerm: Not sure what you mean by that. Are you referring to Graphviz HTML tables? (https://graphviz.
jpienaarUnsubmitted
Not Done
ReplyInline Actions

Yes, an HTML table. But indeed I ran into the same issue with long lines on larger graphs. Duplication is good idea for keeping it scoped - it makes it more difficult to notice when 2 ops have the same input (so if one has patterns that match equal operands then that is more desirable).

jpienaar: Yes, an HTML table. But indeed I ran into the same issue with long lines on larger graphs.
/// Generate a label for an operation.
rriddleUnsubmitted
Done
ReplyInline Actions
emitClusterStmt([&]() {
- for (auto &blockArg : block.getArguments())
+ for (BlockArgument &blockArg : block.getArguments())
valueToNode[blockArg] = emitNodeStmt(getLabel(blockArg));
rriddle:
std::string getLabel(Operation *op) {
return strFromOs([&](raw_ostream &os) {
// Print operation name and type.
os << op->getName() << " : (";
interleaveComma(op->getResultTypes(), os);
os << ")\n";
// Print all the ops in a module. // Print attributes.
rriddleUnsubmitted
Done
ReplyInline Actions
// Print attributes.
- for (auto attr : op->getAttrs()) {
+ for (Attribute attr : op->getAttrs()) {
os << '\n' << attr.first << ": ";
rriddle:
void processModule(ModuleOp module) { for (const NamedAttribute &attr : op->getAttrs()) {
for (Operation &op : module) { os << '\n' << attr.first << ": ";
// Modules may actually be nested, recurse on nesting. emitMlirAttr(os, attr.second);
if (auto nestedModule = dyn_cast<ModuleOp>(op)) {
processModule(nestedModule);
continue;
}
auto opName = getOpName(op);
for (Region &region : op.getRegions()) {
for (auto indexed_block : llvm::enumerate(region)) {
// Suffix block number if there are more than 1 block.
auto blockName = llvm::hasSingleElement(region)
? ""
: ("__" + llvm::utostr(indexed_block.index()));
llvm::WriteGraph(os, &indexed_block.value(), shortNames,
Twine(title) + opName + blockName);
}
}
} }
});
} }
void runOnOperation() override { processModule(getOperation()); } /// Generate a label for a block argument.
std::string getLabel(BlockArgument arg) {
private: return "arg" + std::to_string(arg.getArgNumber());
raw_ostream &os; }
int unnamedOpCtr = 0;
/// Process a block. Emit a cluster and one node per block argument and
/// operation inside the cluster.
void processBlock(Block &block) {
emitClusterStmt([&]() {
for (BlockArgument &blockArg : block.getArguments())
valueToNode[blockArg] = emitNodeStmt(getLabel(blockArg));
// Emit a node for each operation.
for (Operation &op : block)
processOperation(&op);
});
}
/// Process an operation. If the operation has regions, emit a cluster.
/// Otherwise, emit a node.
void processOperation(Operation *op) {
Node node;
if (op->getNumRegions() > 0) {
// Emit cluster for op with regions.
node = emitClusterStmt(
[&]() {
for (Region &region : op->getRegions())
processRegion(region);
},
getLabel(op));
} else {
node = emitNodeStmt(getLabel(op));
}
// Insert edges originating from each operand.
unsigned numOperands = op->getNumOperands();
for (unsigned i = 0; i < numOperands; i++)
emitEdgeStmt(valueToNode[op->getOperand(i)], node,
/*label=*/numOperands == 1 ? "" : std::to_string(i));
for (Value result : op->getResults())
valueToNode[result] = node;
}
/// Process a region.
void processRegion(Region &region) {
for (Block &block : region.getBlocks())
rriddleUnsubmitted
Not Done
ReplyInline Actions
void processRegion(Region &region) {
- for (auto &block : region.getBlocks())
+ for (Block &block : region)
processBlock(block);
rriddle:
processBlock(block);
}
/// Output stream to write DOT file to.
raw_indented_ostream os;
rriddleUnsubmitted
Done
ReplyInline Actions

The llvm:: here shouldn't be necessary.

rriddle: The llvm:: here shouldn't be necessary.
/// A list of edges. For simplicity, should be emitted after all nodes were
rriddleUnsubmitted
Not Done
ReplyInline Actions

Where is plainOs used? It looks like it can be dropped.

rriddle: Where is plainOs used? It looks like it can be dropped.
springermAuthorUnsubmitted
Done
ReplyInline Actions

This is necessary to implement the copy constructor (required by clonePass) correctly. There's no way to copy the indented output stream, so I have to construct a new one. That's where I pass in plainOs.

See my response to jpienaar's comment regarding the indented output stream for more details.

springerm: This is necessary to implement the copy constructor (required by `clonePass`) correctly.
rriddleUnsubmitted
Done
ReplyInline Actions

Can we just add a raw_ostream &getOStream() { return os; } method to raw_indented_ostream then? The use of plainOs feels a bit hacky.

rriddle: Can we just add a `raw_ostream &getOStream() { return os; }` method to raw_indented_ostream…
/// emitted.
std::vector<std::string> edges;
/// Mapping of SSA values to Graphviz nodes/clusters.
DenseMap<Value, Node> valueToNode;
/// Counter for generating unique node/subgraph identifiers.
int counter = 0;
}; };
} // namespace
void mlir::viewGraph(Block &block, const Twine &name, bool shortNames, } // namespace
const Twine &title, llvm::GraphProgram::Name program) {
llvm::ViewGraph(&block, name, shortNames, title, program);
}
raw_ostream &mlir::writeGraph(raw_ostream &os, Block &block, bool shortNames,
const Twine &title) {
return llvm::WriteGraph(os, &block, shortNames, title);
}
std::unique_ptr<OperationPass<ModuleOp>> std::unique_ptr<Pass>
mlir::createPrintOpGraphPass(raw_ostream &os, bool shortNames, mlir::createPrintOpGraphPass(raw_ostream &os) {
const Twine &title) { return std::make_unique<PrintOpPass>(os);
return std::make_unique<PrintOpPass>(os, shortNames, title);
} }

mlir/test/Transforms/print-op-graph.mlir

// RUN: mlir-opt -allow-unregistered-dialect -mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t 2>&1 | FileCheck %s // RUN: mlir-opt -allow-unregistered-dialect -mlir-elide-elementsattrs-if-larger=2 -view-op-graph %s -o %t 2>&1 | FileCheck %s
// CHECK-LABEL: digraph "merge_blocks" // CHECK-LABEL: digraph G {
// CHECK{LITERAL}: value: [[...]] : tensor\<2x2xi32\>} // CHECK: subgraph {{.*}} {
// CHECK{LITERAL}: value: dense\<1\> : tensor\<5xi32\>} // CHECK: subgraph {{.*}}
// CHECK{LITERAL}: value: dense\<[[0, 1]]\> : tensor\<1x2xi32\>} // CHECK: label = "builtin.func{{.*}}merge_blocks
// CHECK: subgraph {{.*}} {
// CHECK: v[[ARG0:.*]] [label = "arg0"
// CHECK: v[[CONST10:.*]] [label ={{.*}}10 : i32
// CHECK: subgraph [[CLUSTER_MERGE_BLOCKS:.*]] {
// CHECK: v[[ANCHOR:.*]] [label = " ", shape = plain]
// CHECK: label = "test.merge_blocks
// CHECK: subgraph {{.*}} {
// CHECK: v[[TEST_BR:.*]] [label = "test.br
// CHECK: }
// CHECK: subgraph {{.*}} {
// CHECK: }
// CHECK: }
// CHECK: v[[TEST_RET:.*]] [label = "test.return
// CHECK: v[[ARG0]] -> v[[TEST_BR]]
// CHECK: v[[CONST10]] -> v[[TEST_BR]]
// CHECK: v[[ANCHOR]] -> v[[TEST_RET]] [{{.*}}, ltail = [[CLUSTER_MERGE_BLOCKS]]]
// CHECK: v[[ANCHOR]] -> v[[TEST_RET]] [{{.*}}, ltail = [[CLUSTER_MERGE_BLOCKS]]]
func @merge_blocks(%arg0: i32, %arg1 : i32) -> () { func @merge_blocks(%arg0: i32, %arg1 : i32) -> () {
%0 = constant dense<[[0, 1], [2, 3]]> : tensor<2x2xi32> %0 = constant dense<[[0, 1], [2, 3]]> : tensor<2x2xi32>
%1 = constant dense<1> : tensor<5xi32> %1 = constant dense<1> : tensor<5xi32>
%2 = constant dense<[[0, 1]]> : tensor<1x2xi32> %2 = constant dense<[[0, 1]]> : tensor<1x2xi32>
%a = constant 10 : i32
%b = "test.func"() : () -> i32
%3:2 = "test.merge_blocks"() ({ %3:2 = "test.merge_blocks"() ({
^bb0: ^bb0:
"test.br"(%arg0, %arg1)[^bb1] : (i32, i32) -> () "test.br"(%arg0, %b, %a)[^bb1] : (i32, i32, i32) -> ()
^bb1(%arg3 : i32, %arg4 : i32): ^bb1(%arg3 : i32, %arg4 : i32, %arg5: i32):
"test.return"(%arg3, %arg4) : (i32, i32) -> () "test.return"(%arg3, %arg4) : (i32, i32) -> ()
}) : () -> (i32, i32) }) : () -> (i32, i32)
"test.return"(%3#0, %3#1) : (i32, i32) -> () "test.return"(%3#0, %3#1) : (i32, i32) -> ()
} }