Use cleaner symbol visibility syntax for flow types. (#6964)
Progress on https://github.com/google/iree/issues/4670
diff --git a/iree/compiler/Dialect/Flow/IR/FlowOps.cpp b/iree/compiler/Dialect/Flow/IR/FlowOps.cpp
index 6513688..4b51012 100644
--- a/iree/compiler/Dialect/Flow/IR/FlowOps.cpp
+++ b/iree/compiler/Dialect/Flow/IR/FlowOps.cpp
@@ -8,6 +8,7 @@
#include "iree/compiler/Dialect/Shape/IR/Builders.h"
#include "iree/compiler/Dialect/Util/IR/ClosureOpUtils.h"
+#include "iree/compiler/Dialect/Util/IR/UtilOps.h"
#include "iree/compiler/Dialect/Util/IR/UtilTypes.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/StringExtras.h"
@@ -531,37 +532,6 @@
builder.getStringAttr(name));
}
-static ParseResult parseExecutableOp(OpAsmParser &parser,
- OperationState *result) {
- StringAttr nameAttr;
- if (failed(parser.parseSymbolName(nameAttr,
- mlir::SymbolTable::getSymbolAttrName(),
- result->attributes)) ||
- failed(parser.parseOptionalAttrDictWithKeyword(result->attributes))) {
- return failure();
- }
-
- // Parse the module body.
- auto *body = result->addRegion();
- if (failed(parser.parseRegion(*body, llvm::None, llvm::None))) {
- return failure();
- }
-
- // Ensure that this module has a valid terminator.
- ExecutableOp::ensureTerminator(*body, parser.getBuilder(), result->location);
- return success();
-}
-
-static void printExecutableOp(OpAsmPrinter &p, ExecutableOp op) {
- p << ' ';
- p.printSymbolName(op.sym_name());
- p.printOptionalAttrDictWithKeyword(
- op->getAttrs(),
- /*elidedAttrs=*/{mlir::SymbolTable::getSymbolAttrName()});
- p.printRegion(op.body(), /*printEntryBlockArgs=*/false,
- /*printBlockTerminators=*/false);
-}
-
static LogicalResult verifyExecutableOp(ExecutableOp op) {
// TODO(benvanik): check export name conflicts.
return success();
@@ -571,8 +541,20 @@
// flow.dispatch.entry
//===----------------------------------------------------------------------===//
+void DispatchEntryOp::build(OpBuilder &builder, OperationState &state,
+ StringRef sym_name, FlatSymbolRefAttr function_ref,
+ IntegerAttr workgroup_rank) {
+ build(builder, state, /*sym_visibility=*/nullptr,
+ builder.getStringAttr(sym_name), function_ref, workgroup_rank);
+}
+
static ParseResult parseDispatchEntryOp(OpAsmParser &parser,
OperationState *result) {
+ StringAttr visibilityAttr;
+ if (failed(parseSymbolVisibility(parser, visibilityAttr))) {
+ return failure();
+ }
+
FlatSymbolRefAttr functionRefAttr;
if (failed(parser.parseAttribute(functionRefAttr, "function_ref",
result->attributes))) {
@@ -601,6 +583,8 @@
static void printDispatchEntryOp(OpAsmPrinter &p, DispatchEntryOp op) {
p << ' ';
+ printSymbolVisibility(p, op, op->getAttrOfType<StringAttr>("sym_visibility"));
+ p << ' ';
p.printSymbolName(op.function_ref());
if (op.sym_name() != op.function_ref()) {
p << " as(\"" << op.sym_name() << "\")";
diff --git a/iree/compiler/Dialect/Flow/IR/FlowOps.td b/iree/compiler/Dialect/Flow/IR/FlowOps.td
index 2df9623..3dbc19f 100644
--- a/iree/compiler/Dialect/Flow/IR/FlowOps.td
+++ b/iree/compiler/Dialect/Flow/IR/FlowOps.td
@@ -460,12 +460,21 @@
}];
let arguments = (ins
- StrAttr:$sym_name
+ OptionalAttr<StrAttr>:$sym_visibility,
+ SymbolNameAttr:$sym_name
// TODO(benvanik): add compatibility and versioning attributes.
);
let regions = (region SizedRegion<1>:$body);
+ let assemblyFormat = [{
+ custom<SymbolVisibility>($sym_visibility)
+ $sym_name
+ attr-dict-with-keyword
+ ``
+ regions
+ }];
+
let skipDefaultBuilders = 1;
let builders = [
OpBuilder<(ins "StringRef":$name)>,
@@ -501,10 +510,18 @@
}];
let arguments = (ins
- StrAttr:$sym_name,
+ OptionalAttr<StrAttr>:$sym_visibility,
+ SymbolNameAttr:$sym_name,
FlatSymbolRefAttr:$function_ref,
OptionalAttr<IndexAttr>:$workgroup_rank
);
+
+ let builders = [
+ OpBuilder<(ins
+ "StringRef":$sym_name,
+ "FlatSymbolRefAttr":$function_ref,
+ "IntegerAttr":$workgroup_rank)>,
+ ];
}
//===----------------------------------------------------------------------===//
diff --git a/iree/compiler/Dialect/Flow/IR/test/executable_ops.mlir b/iree/compiler/Dialect/Flow/IR/test/executable_ops.mlir
index 3369a78..aa053c5 100644
--- a/iree/compiler/Dialect/Flow/IR/test/executable_ops.mlir
+++ b/iree/compiler/Dialect/Flow/IR/test/executable_ops.mlir
@@ -11,8 +11,8 @@
return
}
}
- // CHECK: flow.dispatch.entry @dispatch0
+ // CHECK: flow.dispatch.entry public @dispatch0
flow.dispatch.entry @dispatch0
- // CHECK: flow.dispatch.entry @dispatch0 as("dispatch0_alias")
+ // CHECK: flow.dispatch.entry public @dispatch0 as("dispatch0_alias")
flow.dispatch.entry @dispatch0 as("dispatch0_alias")
}
diff --git a/iree/compiler/Dialect/Flow/Transforms/OutlineDispatchRegions.cpp b/iree/compiler/Dialect/Flow/Transforms/OutlineDispatchRegions.cpp
index 1109361..5f2c570 100644
--- a/iree/compiler/Dialect/Flow/Transforms/OutlineDispatchRegions.cpp
+++ b/iree/compiler/Dialect/Flow/Transforms/OutlineDispatchRegions.cpp
@@ -214,7 +214,7 @@
// Add executable entry point pointing at the function.
OpBuilder builder(executableOp.body());
auto entryPointOp = builder.create<DispatchEntryOp>(
- regionOp.getLoc(), builder.getStringAttr(workgroupFuncOp.getName()),
+ regionOp.getLoc(), workgroupFuncOp.getName(),
SymbolRefAttr::get(workgroupFuncOp),
builder.getIndexAttr(regionOp.getWorkgroupRank()));
diff --git a/iree/compiler/Dialect/Flow/Transforms/test/deduplicate_executables.mlir b/iree/compiler/Dialect/Flow/Transforms/test/deduplicate_executables.mlir
index 37d9878..e58e05b 100644
--- a/iree/compiler/Dialect/Flow/Transforms/test/deduplicate_executables.mlir
+++ b/iree/compiler/Dialect/Flow/Transforms/test/deduplicate_executables.mlir
@@ -1,6 +1,6 @@
// RUN: iree-opt -split-input-file -iree-flow-deduplicate-executables %s | IreeFileCheck %s
-// CHECK-LABEL: flow.executable @single_executable_ex_0
+// CHECK-LABEL: flow.executable public @single_executable_ex_0
flow.executable @single_executable_ex_0 {
flow.dispatch.entry @single_executable_entry_0
builtin.module {
@@ -20,7 +20,7 @@
// -----
-// CHECK-LABEL: flow.executable @duplicate_executables_ex_0
+// CHECK-LABEL: flow.executable public @duplicate_executables_ex_0
flow.executable @duplicate_executables_ex_0 {
flow.dispatch.entry @duplicate_executables_entry_0
builtin.module {
@@ -30,7 +30,7 @@
}
}
}
-// CHECK-NOT: flow.executable @duplicate_executables_ex_1
+// CHECK-NOT: flow.executable public @duplicate_executables_ex_1
flow.executable @duplicate_executables_ex_1 {
flow.dispatch.entry @duplicate_executables_entry_1
builtin.module {
@@ -40,7 +40,7 @@
}
}
}
-// CHECK-LABEL: flow.executable @duplicate_executables_ex_2
+// CHECK-LABEL: flow.executable public @duplicate_executables_ex_2
flow.executable @duplicate_executables_ex_2 {
flow.dispatch.entry @duplicate_executables_entry_2
builtin.module {
@@ -64,7 +64,7 @@
// -----
-// CHECK: flow.executable @same_ops_diff_operands_ex_0
+// CHECK: flow.executable public @same_ops_diff_operands_ex_0
flow.executable @same_ops_diff_operands_ex_0 {
flow.dispatch.entry @entry_0
builtin.module {
@@ -74,7 +74,7 @@
}
}
}
-// CHECK: flow.executable @same_ops_diff_operands_ex_1
+// CHECK: flow.executable public @same_ops_diff_operands_ex_1
flow.executable @same_ops_diff_operands_ex_1 {
flow.dispatch.entry @entry_1
builtin.module {
@@ -96,7 +96,7 @@
// -----
-// CHECK-LABEL: flow.executable @multiple_entry_points_ex_0
+// CHECK-LABEL: flow.executable public @multiple_entry_points_ex_0
flow.executable @multiple_entry_points_ex_0 {
flow.dispatch.entry @multiple_entry_points_0_entry_0
flow.dispatch.entry @multiple_entry_points_0_entry_1
@@ -111,7 +111,7 @@
}
}
}
-// CHECK-NOT: flow.executable @multiple_entry_points_ex_1
+// CHECK-NOT: flow.executable public @multiple_entry_points_ex_1
flow.executable @multiple_entry_points_ex_1 {
flow.dispatch.entry @multiple_entry_points_1_entry_0
flow.dispatch.entry @multiple_entry_points_1_entry_1
@@ -142,7 +142,7 @@
// -----
-// CHECK-LABEL: flow.executable @different_types_float_ex
+// CHECK-LABEL: flow.executable public @different_types_float_ex
flow.executable @different_types_float_ex {
flow.dispatch.entry @different_types_float_entry
builtin.module {
@@ -152,7 +152,7 @@
}
}
}
-// CHECK-LABEL: flow.executable @different_types_int_ex
+// CHECK-LABEL: flow.executable public @different_types_int_ex
flow.executable @different_types_int_ex {
flow.dispatch.entry @different_types_int_entry
builtin.module {
@@ -174,7 +174,7 @@
// -----
-// CHECK-LABEL: flow.executable @nested_ops_ex_0
+// CHECK-LABEL: flow.executable public @nested_ops_ex_0
flow.executable @nested_ops_ex_0 {
flow.dispatch.entry @nested_ops_entry_0
builtin.module {
@@ -189,7 +189,7 @@
}
}
}
-// CHECK-NOT: flow.executable @nested_ops_ex_1
+// CHECK-NOT: flow.executable public @nested_ops_ex_1
flow.executable @nested_ops_ex_1 {
flow.dispatch.entry @nested_ops_entry_1
builtin.module {
@@ -204,7 +204,7 @@
}
}
}
-// CHECK-LABEL: flow.executable @nested_ops_ex_2
+// CHECK-LABEL: flow.executable public @nested_ops_ex_2
flow.executable @nested_ops_ex_2 {
flow.dispatch.entry @nested_ops_entry_2
builtin.module {
@@ -233,7 +233,7 @@
// -----
-// CHECK-LABEL: flow.executable @attributes_ex_0
+// CHECK-LABEL: flow.executable public @attributes_ex_0
flow.executable @attributes_ex_0 {
flow.dispatch.entry @attributes_entry_0
builtin.module {
@@ -249,7 +249,7 @@
}
}
-// CHECK-LABEL: flow.executable @attributes_ex_1
+// CHECK-LABEL: flow.executable public @attributes_ex_1
flow.executable @attributes_ex_1 {
flow.dispatch.entry @attributes_entry_1
builtin.module {
@@ -266,7 +266,7 @@
}
}
// Duplicate of @attributes_ex_0
-// CHECK-NOT: flow.executable @attributes_ex_2
+// CHECK-NOT: flow.executable public @attributes_ex_2
flow.executable @attributes_ex_2 {
flow.dispatch.entry @attributes_entry_2
builtin.module {
@@ -284,7 +284,7 @@
// -----
-// CHECK-LABEL: flow.executable @block_successors_ex_0
+// CHECK-LABEL: flow.executable public @block_successors_ex_0
flow.executable @block_successors_ex_0 {
flow.dispatch.entry @entry_0
builtin.module {
@@ -300,7 +300,7 @@
}
}
}
-// CHECK-LABEL: flow.executable @block_successors_ex_with_swapped_cond_br
+// CHECK-LABEL: flow.executable public @block_successors_ex_with_swapped_cond_br
flow.executable @block_successors_ex_with_swapped_cond_br {
flow.dispatch.entry @entry_1
builtin.module {
diff --git a/iree/compiler/Dialect/Flow/Transforms/test/outline_dispatch_regions.mlir b/iree/compiler/Dialect/Flow/Transforms/test/outline_dispatch_regions.mlir
index e9d0d7e..6e744be 100644
--- a/iree/compiler/Dialect/Flow/Transforms/test/outline_dispatch_regions.mlir
+++ b/iree/compiler/Dialect/Flow/Transforms/test/outline_dispatch_regions.mlir
@@ -1,7 +1,7 @@
// RUN: iree-opt -allow-unregistered-dialect -split-input-file -iree-flow-outline-dispatch-regions2 %s | IreeFileCheck %s
-// CHECK: flow.executable @staticShapeDispatch_dispatch_0
-// CHECK-NEXT: flow.dispatch.entry @staticShapeDispatch_dispatch_0 attributes {
+// CHECK: flow.executable private @staticShapeDispatch_dispatch_0
+// CHECK-NEXT: flow.dispatch.entry public @staticShapeDispatch_dispatch_0 attributes {
// CHECK-SAME: workgroup_rank = 2 : index}
// CHECK: func @staticShapeDispatch_dispatch_0(
// CHECK-SAME: %[[ARG:.+]]: !flow.dispatch.tensor<readonly:8x4xf32>,
@@ -40,13 +40,13 @@
// -----
-// CHECK: flow.executable @dispatchFnMuli_dispatch_0
-// CHECK-NEXT: flow.dispatch.entry @dispatchFnMuli_dispatch_0 attributes {
+// CHECK: flow.executable private @dispatchFnMuli_dispatch_0
+// CHECK-NEXT: flow.dispatch.entry public @dispatchFnMuli_dispatch_0 attributes {
// CHECK-SAME: workgroup_rank = 2 : index}
// CHECK: func @dispatchFnMuli_dispatch_0(
-// CHECK: flow.executable @dispatchFnMuli_dispatch_1
-// CHECK-NEXT: flow.dispatch.entry @dispatchFnMuli_dispatch_1 attributes {
+// CHECK: flow.executable private @dispatchFnMuli_dispatch_1
+// CHECK-NEXT: flow.dispatch.entry public @dispatchFnMuli_dispatch_1 attributes {
// CHECK-SAME: workgroup_rank = 2 : index}
// CHECK: func @dispatchFnMuli_dispatch_1(
@@ -89,7 +89,7 @@
// -----
-// CHECK: flow.executable @dispatchFn1_dispatch_0
+// CHECK: flow.executable private @dispatchFn1_dispatch_0
// CHECK-LABEL: func @dispatchFn1
func @dispatchFn1(%arg0 : tensor<8x4xf32>) -> tensor<4x8xf32> {
@@ -104,7 +104,7 @@
return %0 : tensor<4x8xf32>
}
-// CHECK: flow.executable @dispatchFn2_dispatch_0
+// CHECK: flow.executable private @dispatchFn2_dispatch_0
// CHECK-LABEL: func @dispatchFn2
func @dispatchFn2(%arg0 : tensor<8x4xf32>) -> tensor<4x8xf32> {
@@ -121,8 +121,8 @@
// -----
-// CHECK: flow.executable @dynamicShapeDispatch_dispatch_0
-// CHECK-NEXT: flow.dispatch.entry @dynamicShapeDispatch_dispatch_0 attributes {
+// CHECK: flow.executable private @dynamicShapeDispatch_dispatch_0
+// CHECK-NEXT: flow.dispatch.entry public @dynamicShapeDispatch_dispatch_0 attributes {
// CHECK-SAME: workgroup_rank = 2 : index}
// CHECK: func @dynamicShapeDispatch_dispatch_0(
// CHECK-SAME: %[[ARG:.+]]: !flow.dispatch.tensor<readonly:7x?x24x?xf32>,
diff --git a/iree/compiler/Dialect/Flow/Transforms/test/transformation.mlir b/iree/compiler/Dialect/Flow/Transforms/test/transformation.mlir
index 3c16589..e0c7095 100644
--- a/iree/compiler/Dialect/Flow/Transforms/test/transformation.mlir
+++ b/iree/compiler/Dialect/Flow/Transforms/test/transformation.mlir
@@ -15,8 +15,8 @@
return %2 : tensor<4xf32>
}
-// CHECK-LABEL: flow.executable @hloElementwiseOps_dispatch_0 attributes {sym_visibility = "private"} {
-// CHECK-NEXT: flow.dispatch.entry @hloElementwiseOps_dispatch_0
+// CHECK-LABEL: flow.executable private @hloElementwiseOps_dispatch_0 {
+// CHECK-NEXT: flow.dispatch.entry public @hloElementwiseOps_dispatch_0
// CHECK-NEXT: module {
// CHECK-NEXT: func @hloElementwiseOps_dispatch_0(%arg0: !flow.dispatch.tensor<readonly:4xf32>, %arg1: !flow.dispatch.tensor<writeonly:4xf32>) {
// CHECK: %{{.+}} = linalg.generic
@@ -43,19 +43,19 @@
return %2 : tensor<4x4xf32>
}
-// CHECK-LABEL: flow.executable @interleavedDot_dispatch_0 attributes {sym_visibility = "private"} {
-// CHECK-NEXT: flow.dispatch.entry @interleavedDot_dispatch_0
+// CHECK-LABEL: flow.executable private @interleavedDot_dispatch_0 {
+// CHECK-NEXT: flow.dispatch.entry public @interleavedDot_dispatch_0
// CHECK-NEXT: module {
// CHECK-NEXT: func @interleavedDot_dispatch_0
// CHECK: %{{.+}} = linalg.generic
// CHECK: %{{.+}} = addf %{{.+}}, %{{.+}} : f32
-// CHECK: flow.executable @interleavedDot_dispatch_1 attributes {sym_visibility = "private"} {
-// CHECK-NEXT: flow.dispatch.entry @interleavedDot_dispatch_1
+// CHECK: flow.executable private @interleavedDot_dispatch_1 {
+// CHECK-NEXT: flow.dispatch.entry public @interleavedDot_dispatch_1
// CHECK-NEXT: module {
// CHECK-NEXT: func @interleavedDot_dispatch_1
// CHECK: %{{.+}} = linalg.matmul
-// CHECK: flow.executable @interleavedDot_dispatch_2 attributes {sym_visibility = "private"} {
-// CHECK-NEXT: flow.dispatch.entry @interleavedDot_dispatch_2
+// CHECK: flow.executable private @interleavedDot_dispatch_2 {
+// CHECK-NEXT: flow.dispatch.entry public @interleavedDot_dispatch_2
// CHECK-NEXT: module {
// CHECK-NEXT: func @interleavedDot_dispatch_2
// CHECK: %{{.+}} = linalg.generic
@@ -85,8 +85,8 @@
return %1 : tensor<4xf32>
}
-// CHECK-LABEL: flow.executable @reduction_dispatch_0 attributes {sym_visibility = "private"} {
-// CHECK-NEXT: flow.dispatch.entry @reduction_dispatch_0
+// CHECK-LABEL: flow.executable private @reduction_dispatch_0 {
+// CHECK-NEXT: flow.dispatch.entry public @reduction_dispatch_0
// CHECK-NEXT: module {
// CHECK-NEXT: func @reduction_dispatch_0
// CHECK: %{{.+}} = linalg.generic
