[mlir] Handle cycles and back edges in --view-op-graph #82002

agentcooper · 2024-02-16T15:45:38Z

Fixes #62128.

Before:

After:

llvmbot · 2024-02-16T15:46:11Z

@llvm/pr-subscribers-mlir

Author: Artem Tyurin (agentcooper)

Changes

Fixes #62128.

Before:

After:

Full diff: https://github.com/llvm/llvm-project/pull/82002.diff

2 Files Affected:

(modified) mlir/lib/Transforms/ViewOpGraph.cpp (+3)
(added) mlir/test/Transforms/print-op-graph-backedges.mlir (+24)

diff --git a/mlir/lib/Transforms/ViewOpGraph.cpp b/mlir/lib/Transforms/ViewOpGraph.cpp
index 3d2723839957c4..485f93aeb9c3c4 100644
--- a/mlir/lib/Transforms/ViewOpGraph.cpp
+++ b/mlir/lib/Transforms/ViewOpGraph.cpp
@@ -13,6 +13,7 @@
 #include "mlir/IR/Operation.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Support/IndentedOstream.h"
+#include "mlir/Transforms/TopologicalSortUtils.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/GraphWriter.h"
 #include <map>
@@ -276,6 +277,8 @@ class PrintOpPass : public impl::ViewOpGraphBase<PrintOpPass> {
   /// Process a block. Emit a cluster and one node per block argument and
   /// operation inside the cluster.
   void processBlock(Block &block) {
+    sortTopologically(&block);
+
     emitClusterStmt([&]() {
       for (BlockArgument &blockArg : block.getArguments())
         valueToNode[blockArg] = emitNodeStmt(getLabel(blockArg));
diff --git a/mlir/test/Transforms/print-op-graph-backedges.mlir b/mlir/test/Transforms/print-op-graph-backedges.mlir
new file mode 100644
index 00000000000000..cd03bd0c1c298f
--- /dev/null
+++ b/mlir/test/Transforms/print-op-graph-backedges.mlir
@@ -0,0 +1,24 @@
+// RUN: mlir-opt -view-op-graph %s -o %t 2>&1 | FileCheck -check-prefix=DFG %s
+
+// DFG-LABEL: digraph G {
+//       DFG:   compound = true;
+//       DFG:   subgraph cluster_1 {
+//       DFG:     v2 [label = " ", shape = plain];
+//       DFG:     label = "builtin.module : ()\n";
+//       DFG:     subgraph cluster_3 {
+//       DFG:       v4 [label = " ", shape = plain];
+//       DFG:       label = "";
+//       DFG:       v5 [fillcolor = "0.333333 1.0 1.0", label = "arith.constant : (index)\n\nvalue: 0 : index", shape = ellipse, style = filled];
+//       DFG:       v6 [fillcolor = "0.333333 1.0 1.0", label = "arith.constant : (index)\n\nvalue: 1 : index", shape = ellipse, style = filled];
+//       DFG:       v7 [fillcolor = "0.000000 1.0 1.0", label = "arith.addi : (index)\n\noverflowFlags: #arith.overflow<none...", shape = ellipse, style = filled];
+//       DFG:     }
+//       DFG:   }
+//       DFG:   v5 -> v7 [label = "0", style = solid];
+//       DFG:   v6 -> v7 [label = "1", style = solid];
+//       DFG: }
+
+module {
+  %add = arith.addi %c0, %c1 : index
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+}

llvmbot · 2024-02-16T15:46:11Z

@llvm/pr-subscribers-mlir-core

Author: Artem Tyurin (agentcooper)

Changes

Fixes #62128.

Before:

After:

Full diff: https://github.com/llvm/llvm-project/pull/82002.diff

2 Files Affected:

(modified) mlir/lib/Transforms/ViewOpGraph.cpp (+3)
(added) mlir/test/Transforms/print-op-graph-backedges.mlir (+24)

diff --git a/mlir/lib/Transforms/ViewOpGraph.cpp b/mlir/lib/Transforms/ViewOpGraph.cpp
index 3d2723839957c4..485f93aeb9c3c4 100644
--- a/mlir/lib/Transforms/ViewOpGraph.cpp
+++ b/mlir/lib/Transforms/ViewOpGraph.cpp
@@ -13,6 +13,7 @@
 #include "mlir/IR/Operation.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Support/IndentedOstream.h"
+#include "mlir/Transforms/TopologicalSortUtils.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/GraphWriter.h"
 #include <map>
@@ -276,6 +277,8 @@ class PrintOpPass : public impl::ViewOpGraphBase<PrintOpPass> {
   /// Process a block. Emit a cluster and one node per block argument and
   /// operation inside the cluster.
   void processBlock(Block &block) {
+    sortTopologically(&block);
+
     emitClusterStmt([&]() {
       for (BlockArgument &blockArg : block.getArguments())
         valueToNode[blockArg] = emitNodeStmt(getLabel(blockArg));
diff --git a/mlir/test/Transforms/print-op-graph-backedges.mlir b/mlir/test/Transforms/print-op-graph-backedges.mlir
new file mode 100644
index 00000000000000..cd03bd0c1c298f
--- /dev/null
+++ b/mlir/test/Transforms/print-op-graph-backedges.mlir
@@ -0,0 +1,24 @@
+// RUN: mlir-opt -view-op-graph %s -o %t 2>&1 | FileCheck -check-prefix=DFG %s
+
+// DFG-LABEL: digraph G {
+//       DFG:   compound = true;
+//       DFG:   subgraph cluster_1 {
+//       DFG:     v2 [label = " ", shape = plain];
+//       DFG:     label = "builtin.module : ()\n";
+//       DFG:     subgraph cluster_3 {
+//       DFG:       v4 [label = " ", shape = plain];
+//       DFG:       label = "";
+//       DFG:       v5 [fillcolor = "0.333333 1.0 1.0", label = "arith.constant : (index)\n\nvalue: 0 : index", shape = ellipse, style = filled];
+//       DFG:       v6 [fillcolor = "0.333333 1.0 1.0", label = "arith.constant : (index)\n\nvalue: 1 : index", shape = ellipse, style = filled];
+//       DFG:       v7 [fillcolor = "0.000000 1.0 1.0", label = "arith.addi : (index)\n\noverflowFlags: #arith.overflow<none...", shape = ellipse, style = filled];
+//       DFG:     }
+//       DFG:   }
+//       DFG:   v5 -> v7 [label = "0", style = solid];
+//       DFG:   v6 -> v7 [label = "1", style = solid];
+//       DFG: }
+
+module {
+  %add = arith.addi %c0, %c1 : index
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+}

joker-eph · 2024-02-16T21:46:48Z

mlir/lib/Transforms/ViewOpGraph.cpp

@@ -276,6 +277,8 @@ class PrintOpPass : public impl::ViewOpGraphBase<PrintOpPass> {
  /// Process a block. Emit a cluster and one node per block argument and
  /// operation inside the cluster.
  void processBlock(Block &block) {
+    sortTopologically(&block);


Is the printer not modifying the code and this would change this invariant? This can be undesirable in the context of calling this as debugging helper at various stages of a pass pipeline

Good point, I didn't consider that this pass can be used in the middle of the pipeline. I've updated the code so that it does not modify the existing block.

github-actions · 2024-02-16T23:27:26Z

✅ With the latest revision this PR passed the C/C++ code formatter.

joker-eph

Thanks!

We should just figure out a commit message that makes sense for git when merging :)

joker-eph · 2024-02-17T02:14:23Z

By the way: I'm not sure this is enough to fix the bug, a complete solution would handle cyclic graphs I think?

agentcooper · 2024-02-17T13:12:01Z

@joker-eph I gave it another try and it seems that now it can handle cycles as well.

Example from MLIR Language Reference:

"test.graph_region"() ({ // A Graph region
  %1 = "op1"(%1, %3) : (i32, i32) -> (i32)  // OK: %1, %3 allowed here
  %2 = "test.ssacfg_region"() ({
     %5 = "op2"(%1, %2, %3, %4) : (i32, i32, i32, i32) -> (i32) // OK: %1, %2, %3, %4 all defined in the containing region
  }) : () -> (i32)
  %3 = "op2"(%1, %4) : (i32, i32) -> (i32)  // OK: %4 allowed here
  %4 = "op3"(%1) : (i32) -> (i32)
}) : () -> ()

joker-eph · 2024-02-17T21:37:25Z

Nice!

[mlir] Handle backedges in --view-op-graph

8dc6364

llvmbot added mlir:core MLIR Core Infrastructure mlir labels Feb 16, 2024

joker-eph reviewed Feb 16, 2024

View reviewed changes

agentcooper mentioned this pull request Feb 16, 2024

[MLIR] --view-op-graph doesn't handle graph regions (backedges) #62128

Closed

agentcooper added 2 commits February 17, 2024 00:24

Do not modify existing blocks

96abe87

Format code

51ed372

agentcooper requested a review from joker-eph February 16, 2024 23:37

joker-eph approved these changes Feb 17, 2024

View reviewed changes

Handle cycles

e22ffd6

agentcooper changed the title ~~[mlir] Handle backedges in --view-op-graph~~ [mlir] Handle cycles and back edges in --view-op-graph Feb 17, 2024

agentcooper requested a review from joker-eph February 17, 2024 13:17

joker-eph merged commit 3bef17e into llvm:main Feb 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir] Handle cycles and back edges in --view-op-graph #82002

[mlir] Handle cycles and back edges in --view-op-graph #82002

Uh oh!

agentcooper commented Feb 16, 2024

Uh oh!

llvmbot commented Feb 16, 2024

Uh oh!

llvmbot commented Feb 16, 2024

Uh oh!

joker-eph Feb 16, 2024

Uh oh!

agentcooper Feb 16, 2024

Uh oh!

github-actions bot commented Feb 16, 2024 •

edited

Loading

Uh oh!

joker-eph left a comment

Uh oh!

joker-eph commented Feb 17, 2024

Uh oh!

agentcooper commented Feb 17, 2024

Uh oh!

joker-eph commented Feb 17, 2024

Uh oh!

Uh oh!

[mlir] Handle cycles and back edges in --view-op-graph #82002

[mlir] Handle cycles and back edges in --view-op-graph #82002

Uh oh!

Conversation

agentcooper commented Feb 16, 2024

Uh oh!

llvmbot commented Feb 16, 2024

Uh oh!

llvmbot commented Feb 16, 2024

Uh oh!

joker-eph Feb 16, 2024

Choose a reason for hiding this comment

Uh oh!

agentcooper Feb 16, 2024

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joker-eph left a comment

Choose a reason for hiding this comment

Uh oh!

joker-eph commented Feb 17, 2024

Uh oh!

agentcooper commented Feb 17, 2024

Uh oh!

joker-eph commented Feb 17, 2024

Uh oh!

Uh oh!

github-actions bot commented Feb 16, 2024 •

edited

Loading