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
702–711

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
22

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

85–86

Please use proper C++ constructor field initialization.

421

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
23

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.

75

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)

78

We have an indented ostream, why not use that?

262

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

276

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
75

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.

78

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.
262

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

276

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
78

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]).

102

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

276

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
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
70–71

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

77–79
80
82

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

289
352
411
422

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
422

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
422

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/
Transforms/
15 lines
14 lines
lib/
Transforms/
454 lines
test/
Transforms/
33 lines

Diff 359675

mlir/include/mlir/Transforms/Passes.td

Show First 20 Lines • Show All 682 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", "ModuleOp"> {
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 view of module";
let description = [{ let description = [{
This pass prints a graphviz per block of a module. This pass prints a graphviz per block of a module.
- Op are represented as nodes; - Op are represented as nodes;
- Uses as edges; - Uses as edges;
- Regions/blocks as subgraphs;
}]; }];
let constructor = "mlir::createPrintOpGraphPass()"; let constructor = "mlir::createPrintOpGraphPass()";
let options = [ let options = [
Option<"title", "title", "std::string", Option<"color", "color", "bool",
/*default=*/"", "The prefix of the title of the graph">, /*default=*/"true", "Use colors">,
Option<"shortNames", "short-names", "bool", /*default=*/"false", Option<"controlFlowEdges", "control-flow-edges", "bool",
"Use short names"> /*default=*/"false", "Draw control flow edges">,
Option<"duplicateConstNodes", "duplicate-const-nodes", "bool",
/*default=*/"true", "Duplicate nodes for constant ops/block args">,
Option<"maxLocationLen", "max-location-len", "int",
/*default=*/"0", "After how many chars to truncate location attrs">,
Option<"printTypes", "print-types", "bool",
/*default=*/"true", "Print operation types">
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 ModuleOp; class ModuleOp;
template <typename T> class OperationPass; 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<OperationPass<ModuleOp>>
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/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/BuiltinTypes.h"
#include "mlir/IR/Matchers.h"
#include "mlir/IR/Operation.h" #include "mlir/IR/Operation.h"
#include "llvm/Support/CommandLine.h" #include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Format.h"
#include "llvm/Support/FormatVariadic.h"
using namespace mlir; using namespace mlir;
namespace {
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.
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 char kSolidLineStyle[] = "solid";
static const char kDashedLineStyle[] = "dashed";
static const char kEllipseNodeShape[] = "ellipse";
static const char kNoneNodeShape[] = "none";
static const char kColorWhite[] = "#FFFFFF";
static const char *kColors[] = {"#FFADAD", "#FFD6A5", "#FDFFB6", "#CAFFBF",
"#9BF6FF", "#A0C4FF", "#BDB2FF", "#FFC6FF"};
/// 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 { /// Escape special characters such as '\n' and quotation marks.
static std::string escapeString(std::string str) {
std::string buf;
llvm::raw_string_ostream os(buf);
os.write_escaped(str);
return os.str();
}
/// Put quotation marks around a given string.
static std::string quoteString(std::string str) { return "\"" + str + "\""; }
using AttributeMap = llvm::StringMap<std::string>;
/// This struct represents a node in the DOT language. Each node has an
/// identifier and an optional identifier for the cluster (subgraph) that
/// contains the node.
class Node {
public:
Node() : Node(0) {}
static Node create() { return Node(getNextId()); }
static Node createWithCluster() { return Node(getNextId(), getNextId()); }
static int getNextId() {
static int counter = 0;
return ++counter;
}
operator bool() { return id != 0; }
// Specialize GraphTraits to treat Block as a graph of Operations as nodes and int getClusterId() {
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?
// uses as edges. assert(hasCluster());
template <> struct GraphTraits<Block *> { return *clusterId;
using GraphType = Block *;
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) {
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…
return nodes_iterator(b->end(), &AddressOf); int getId() {
assert(id != 0);
return id;
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…
} }
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:
};
rriddleUnsubmitted
Done
ReplyInline Actions
void emitAllEdgeStmts() {
- for (const auto &edge : edges)
+ for (StringRef edge : edges)
os << edge << ";\n";
rriddle:
// Specialize DOTGraphTraits to produce more readable output. bool hasCluster() { return clusterId.hasValue(); }
template <> struct DOTGraphTraits<Block *> : public DefaultDOTGraphTraits { void setId(int id) { this->id = id; }
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.
using DefaultDOTGraphTraits::DefaultDOTGraphTraits;
static std::string getNodeLabel(Operation *op, Block *); private:
Node(int id) { this->id = id; }
Node(int id, int clusterId) : Node(id) { this->clusterId = clusterId; }
rriddleUnsubmitted
Done
ReplyInline Actions

Please use proper C++ constructor field initialization.

rriddle: Please use proper C++ constructor field initialization.
int id;
llvm::Optional<int> clusterId;
}; };
std::string DOTGraphTraits<Block *>::getNodeLabel(Operation *op, Block *b) { static Optional<Attribute> getConstantAttribute(Value val) {
// Reuse the print output for the node labels. Attribute attr;
std::string ostr; if (matchPattern(val, m_Constant(&attr)))
raw_string_ostream os(ostr); return attr;
os << op->getName() << "\n"; return llvm::None;
}
if (!op->getLoc().isa<UnknownLoc>()) { class PrintOpPass : public ViewOpGraphPassBase<PrintOpPass> {
os << op->getLoc() << "\n"; public:
PrintOpPass(raw_ostream &os) : os(os) {}
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…
void runOnOperation() override {
emitGraph([&]() {
processModule(getOperation());
emitAllEdgeStmts();
});
} }
// Print resultant types private:
llvm::interleaveComma(op->getResultTypes(), os); /// Emit output with the current indentation.
os << "\n"; raw_ostream &emitIndented() {
os.indent(2 * indent);
return os;
}
/// Emit all edges. This function should be called after all nodes have been
/// emitted.
void emitAllEdgeStmts() {
for (const auto &edge : edges)
emitIndented() << edge << ";\n";
edges.clear();
}
/// Emit a cluster (subgraph). The specified builder generates the body of the
/// cluster. If the specified node has no cluster, the builder is called
/// directly.
void emitCluster(Node node, function_ref<void()> builder,
std::string label = "", std::string color = kColorWhite) {
if (node.hasCluster()) {
emitIndented() << "subgraph cluster_" << node.getClusterId() << " {\n";
++indent;
emitIndented() << attrStmt("label", quoteString(escapeString(label)))
<< ";\n";
if (this->color) {
emitIndented() << attrStmt("style", "filled") << ";\n";
emitIndented() << attrStmt("fillcolor", quoteString(color)) << ";\n";
}
}
builder();
if (node.hasCluster()) {
--indent;
emitIndented() << "}\n";
}
}
/// Emit an edge from an MLIR value to a Graphviz node. In most cases, the
/// value's node is looked up in `valueToNode`. In a few special cases, a new
/// Graphviz node is created on-the-fly.
/// Note: Edges are appended to a list of edge and written to the output
/// stream via `emitAllEdgeStmts`.
void emitEdgeStmt(Value value, Node node, std::string label,
std::string style = kSolidLineStyle) {
// If `duplicateConstNodes` create nodes on-the-fly for each use.
if (this->duplicateConstNodes) {
// Create a new node for each block argument.
if (auto blockArg = value.dyn_cast<BlockArgument>()) {
Node src = Node::create();
emitNodeStmt(src, getLabel(blockArg), kColorWhite, kNoneNodeShape);
emitEdgeStmt(src, node, label, style);
return;
}
// Create a new node whenever the constant is used.
if (auto attr = getConstantAttribute(value)) {
Node src = Node::create();
std::string buf;
llvm::raw_string_ostream os(buf);
emitMlirAttr(os, *attr);
emitNodeStmt(src, os.str(), kColorWhite, kNoneNodeShape);
emitEdgeStmt(src, node, label, style);
return;
}
}
// Default case: Lookup node in `valueToNode`.
emitEdgeStmt(valueToNode[value], node, label, style);
}
/// Generate an attribute statement.
std::string attrStmt(std::string key, std::string value) {
return key + " = " + value;
}
/// Emit an attribute list.
void emitAttrList(raw_ostream &os, const AttributeMap &map) {
SmallVector<std::string> attrs;
for (auto it = map.begin(); it != map.end(); ++it)
attrs.push_back(attrStmt(it->getKey().str(), it->getValue()));
os << "[";
llvm::interleaveComma(attrs, os);
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();
/// Append an edge to the list of edges.
/// Note: Edges are written to the output stream via `emitAllEdgeStmts`.
void emitEdgeStmt(Node n1, Node n2, std::string label,
std::string style = kSolidLineStyle) {
std::string buf;
llvm::raw_string_ostream edge(buf);
AttributeMap attrs;
attrs["style"] = style;
attrs["label"] = quoteString(escapeString(label));
if (n1.hasCluster())
attrs["ltail"] = "cluster_" + std::to_string(n1.getClusterId());
if (n2.hasCluster())
attrs["lhead"] = "cluster_" + std::to_string(n2.getClusterId());
edge << llvm::format("v%i -> v%i ", n1.getId(), n2.getId());
emitAttrList(edge, attrs);
edges.push_back(edge.str());
}
/// Emit a graph. The specified builder generates the body of the graph.
void emitGraph(function_ref<void()> builder) {
emitIndented() << "digraph G {\n";
++indent;
emitIndented() << attrStmt("compound", "true") << ";\n";
emitIndented() << "node [style = filled];\n";
builder();
--indent;
emitIndented() << "}\n";
}
/// Emit a node statement.
void emitNodeStmt(Node node, std::string label,
std::string color = kColorWhite,
std::string shape = kEllipseNodeShape) {
AttributeMap attrs;
attrs["label"] = quoteString(escapeString(label));
attrs["shape"] = shape;
if (this->color)
attrs["fillcolor"] = quoteString(color);
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…
emitIndented() << llvm::format("v%i ", node.getId());
emitAttrList(os, attrs);
os << ";\n";
} }
} // end namespace llvm /// Generate a label for an operation.
std::string getLabel(Operation *op) {
// Reuse the print output for the node labels.
std::string buf;
llvm::raw_string_ostream os(buf);
namespace { // Print operation name and type.
// PrintOpPass is simple pass to write graph per function. os << op->getName();
// Note: this is a module pass only to avoid interleaving on the same ostream if (this->printTypes) {
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.
// due to multi-threading over functions. os << " : (";
class PrintOpPass : public ViewOpGraphPassBase<PrintOpPass> { // Print result types
public: llvm::interleaveComma(op->getResultTypes(), os);
PrintOpPass(raw_ostream &os, bool shortNames, const Twine &title) : os(os) { os << ")";
this->shortNames = shortNames;
this->title = title.str();
} }
os << "\n";
std::string getOpName(Operation &op) { // Print location.
auto symbolAttr = int maxLocLen = this->maxLocationLen;
op.getAttrOfType<StringAttr>(SymbolTable::getSymbolAttrName()); if (maxLocLen > 0 && !op->getLoc().isa<UnknownLoc>()) {
if (symbolAttr) std::string locStr = llvm::formatv("{0}", op->getLoc()).str();
return std::string(symbolAttr.getValue()); os << "loc: ";
++unnamedOpCtr; // Shorten large loc values to keep the graph readable.
rriddleUnsubmitted
Done
ReplyInline Actions
// Print attributes.
- for (auto attr : op->getAttrs()) {
+ for (Attribute attr : op->getAttrs()) {
os << '\n' << attr.first << ": ";
rriddle:
return (op.getName().getStringRef() + llvm::utostr(unnamedOpCtr)).str(); if (locStr.length() >= maxLocLen) {
os << "..." << locStr.substr(locStr.length() - maxLocLen, maxLocLen);
} else {
os << locStr;
}
os << "\n";
} }
// Print all the ops in a module. // Print attributes.
void processModule(ModuleOp module) { for (auto 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()) { return os.str();
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);
} }
/// Generate a label for a block argument.
std::string getLabel(BlockArgument arg) {
return "arg" + std::to_string(arg.getArgNumber());
} }
/// Return the node color for a given operation. Operations with the same name
/// have the same color.
std::string getColor(Operation *op) {
if (!this->color)
return kColorWhite;
auto opName = op->getName().getStringRef();
static unsigned nextColor = 0;
static llvm::StringMap<std::string> opColors;
if (opColors.find(opName) == opColors.end()) {
opColors[opName] = kColors[nextColor];
nextColor = (nextColor + 1) % (sizeof(kColors) / sizeof(char *));
} }
return opColors[opName];
} }
void runOnOperation() override { processModule(getOperation()); } /// Process a block. Emit a cluster and one node per block argument and
/// operation inside the cluster.
Optional<Node> processBlock(Block &block) {
Node node = Node::createWithCluster();
private: emitCluster(node, [&]() {
raw_ostream &os; // If `duplicateConstNodes`, create block argument nodes on-the-fly.
int unnamedOpCtr = 0; if (!this->duplicateConstNodes) {
}; for (auto &blockArg : block.getArguments()) {
} // namespace Node argNode = Node::create();
valueToNode[blockArg] = argNode;
emitNodeStmt(argNode, getLabel(blockArg));
}
}
void mlir::viewGraph(Block &block, const Twine &name, bool shortNames, // Emit a node for each operation and insert control flow edges between
const Twine &title, llvm::GraphProgram::Name program) { // them.
llvm::ViewGraph(&block, name, shortNames, title, program); Optional<Node> prevNode;
for (auto &op : block.getOperations()) {
if (Optional<Node> opNode = processOperation(&op)) {
node.setId(opNode->getId());
if (this->controlFlowEdges && prevNode && opNode)
emitEdgeStmt(*prevNode, *opNode, "", kDashedLineStyle);
prevNode = opNode;
} }
rriddleUnsubmitted
Done
ReplyInline Actions
emitClusterStmt([&]() {
- for (auto &blockArg : block.getArguments())
+ for (BlockArgument &blockArg : block.getArguments())
valueToNode[blockArg] = emitNodeStmt(getLabel(blockArg));
rriddle:
}
});
raw_ostream &mlir::writeGraph(raw_ostream &os, Block &block, bool shortNames, return node ? Optional<Node>(node) : llvm::None;
const Twine &title) { }
return llvm::WriteGraph(os, &block, shortNames, title);
// Process a module. The contents of the module is wrapped in a cluster.
void processModule(ModuleOp module) {
emitCluster(Node::createWithCluster(), [&]() {
for (Operation &op : module) {
if (auto nestedModule = dyn_cast<ModuleOp>(op))
processModule(nestedModule);
else
processOperation(&op);
} }
});
}
// Process an operation. If the operation has regions, emit a cluster.
// Otherwise, emit a node.
Optional<Node> processOperation(Operation *op) {
// If `duplicateConstNodes`, do not emit nodes for constants. Instead,
// generate a new node, each time it is used in an edge.
if (this->duplicateConstNodes && op->getNumResults() == 1 &&
getConstantAttribute(op->getResult(0)))
return llvm::None;
Node node;
if (op->getNumRegions() > 0) {
// Emit cluster for op with regions.
node = Node::createWithCluster();
emitCluster(
node,
[&]() {
for (Region &region : op->getRegions())
if (auto nestedNode = processRegion(region))
node.setId(nestedNode->getId());
},
getLabel(op), getColor(op));
} else {
node = Node::create();
emitNodeStmt(node, getLabel(op), getColor(op));
}
// Insert edges originating from each operand.
unsigned numOperands = op->getNumOperands();
for (unsigned i = 0; i < numOperands; i++)
emitEdgeStmt(op->getOperand(i), node,
/*label=*/numOperands == 1 ? "" : std::to_string(i));
for (Value result : op->getResults())
valueToNode[result] = node;
return node;
}
// Process a region.
Optional<Node> processRegion(Region &region) {
Optional<Node> result;
rriddleUnsubmitted
Not Done
ReplyInline Actions
void processRegion(Region &region) {
- for (auto &block : region.getBlocks())
+ for (Block &block : region)
processBlock(block);
rriddle:
for (auto &block : region.getBlocks())
if (auto node = processBlock(block))
result = node;
return result;
}
unsigned indent = 0;
raw_ostream &os;
std::vector<std::string> edges;
llvm::DenseMap<Value, Node> valueToNode;
rriddleUnsubmitted
Done
ReplyInline Actions

The llvm:: here shouldn't be necessary.

rriddle: The llvm:: here shouldn't be necessary.
};
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…
} // namespace
std::unique_ptr<OperationPass<ModuleOp>> std::unique_ptr<OperationPass<ModuleOp>>
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='color=true' %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 = "func{{.*}}merge_blocks
// CHECK: subgraph {{.*}} {
// CHECK: subgraph [[CLUSTER_MERGE_BLOCKS:.*]] {
// CHECK: label = "test.merge_blocks
// CHECK: subgraph {{.*}} {
// CHECK: v[[TEST_BR:.*]] [label = "test.br
// CHECK: v[[ARG0:.*]] [label = "arg0"
// CHECK: v[[CONST10:.*]] [label = "10 : i32"
// 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{{.*}} -> v[[TEST_RET]] [{{.*}}, ltail = [[CLUSTER_MERGE_BLOCKS]]]
// CHECK: v{{.*}} -> 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) -> ()
} }