[Mlir-commits] [mlir] [mlir][Interfaces][NFC] Update doc of ViewLikeOpInterface parser/printer handlers (PR #122555)
Diego Caballero
llvmlistbot at llvm.org
Mon Jan 13 10:55:48 PST 2025
https://github.com/dcaballe updated https://github.com/llvm/llvm-project/pull/122555
>From 27eb15c52ab0fdd7a654611f30818521c39a4d01 Mon Sep 17 00:00:00 2001
From: Diego Caballero <dieg0ca6aller0 at gmail.com>
Date: Fri, 10 Jan 2025 15:34:19 -0800
Subject: [PATCH 1/2] [mlir][Interfaces][NFC] Update doc of ViewLikeOpInterface
parser/printer handlers.
This PR addresses part of the feedback provided in #115808.
---
.../mlir/Interfaces/ViewLikeInterface.h | 103 ++++++++++++------
1 file changed, 67 insertions(+), 36 deletions(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index eb046bc742298f..07c6465063dd30 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -86,24 +86,46 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
}
};
-/// Printer hook for custom directive in assemblyFormat.
+/// Printer hooks for custom directive in assemblyFormat.
///
/// custom<DynamicIndexList>($values, $integers)
/// custom<DynamicIndexList>($values, $integers, type($values))
///
-/// where `values` is of ODS type `Variadic<*>` and `integers` is of ODS
-/// type `I64ArrayAttr`. Prints a list with either (1) the static integer value
-/// in `integers` is `kDynamic` or (2) the next value otherwise. If `valueTypes`
-/// is non-empty, it is expected to contain as many elements as `values`
-/// indicating their types. This allows idiomatic printing of mixed value and
-/// integer attributes in a list. E.g.
-/// `[%arg0 : index, 7, 42, %arg42 : i32]`.
-///
-/// Indices can be scalable. For example, "4" in "[2, [4], 8]" is scalable.
-/// This notation is similar to how scalable dims are marked when defining
-/// Vectors. For each value in `integers`, the corresponding `bool` in
-/// `scalables` encodes whether it's a scalable index. If `scalableVals` is
-/// empty then assume that all indices are non-scalable.
+/// where `values` is of ODS type `Variadic<*>` and `integers` is of ODS type
+/// `I64ArrayAttr`. Print a list where each element is either:
+/// 1. the static integer value in `integers`, if it's not `kDynamic` or,
+/// 2. the next value in `values`, otherwise.
+///
+/// If `valueTypes` is provided, the corresponding type of each dynamic value is
+/// printed. Otherwise, the type is not printed. Each type must match the type
+/// of the corresponding value in `values`. The type for integer elements is
+/// `i64` by default and never printed.
+///
+/// Integer indices can also be scalable, denoted with square brackets (e.g.,
+/// "[2, [4], 8]"). For each value in `integers`, the corresponding `bool` in
+/// `scalables` encodes whether it's a scalable index. If `scalables` is empty
+/// then assume that all indices are non-scalable.
+///
+/// Examples:
+///
+/// * Input: `integers = [kDynamic, 7, 42, kDynamic]`,
+/// `values = [%arg0, %arg42]` and
+/// `valueTypes = [index, index]`
+/// prints:
+/// `[%arg0 : index, 7, 42, %arg42 : i32]`
+///
+/// * Input: `integers = [kDynamic, 7, 42, kDynamic]`,
+/// `values = [%arg0, %arg42]` and
+/// `valueTypes = []`
+/// prints:
+/// `[%arg0, 7, 42, %arg42]`
+///
+/// * Input: `integers = [2, 4, 8]`,
+/// `values = []` and
+/// `scalables = [false, true, false]`
+/// prints:
+/// `[2, [4], 8]`
+///
void printDynamicIndexList(
OpAsmPrinter &printer, Operation *op, OperandRange values,
ArrayRef<int64_t> integers, ArrayRef<bool> scalables,
@@ -113,35 +135,44 @@ inline void printDynamicIndexList(
OpAsmPrinter &printer, Operation *op, OperandRange values,
ArrayRef<int64_t> integers, TypeRange valueTypes = TypeRange(),
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square) {
- return printDynamicIndexList(printer, op, values, integers, {}, valueTypes,
- delimiter);
+ return printDynamicIndexList(printer, op, values, integers, /*scalables=*/{},
+ valueTypes, delimiter);
}
-/// Parser hook for custom directive in assemblyFormat.
+/// Parser hooks for custom directive in assemblyFormat.
///
/// custom<DynamicIndexList>($values, $integers)
/// custom<DynamicIndexList>($values, $integers, type($values))
///
/// where `values` is of ODS type `Variadic<*>` and `integers` is of ODS
-/// type `I64ArrayAttr`. Parse a mixed list with either (1) static integer
-/// values or (2) SSA values. Fill `integers` with the integer ArrayAttr, where
-/// `kDynamic` encodes the position of SSA values. Add the parsed SSA values
-/// to `values` in-order. If `valueTypes` is non-null, fill it with types
-/// corresponding to values; otherwise the caller must handle the types.
-///
-/// E.g. after parsing "[%arg0 : index, 7, 42, %arg42 : i32]":
-/// 1. `result` is filled with the i64 ArrayAttr "[`kDynamic`, 7, 42,
-/// `kDynamic`]"
-/// 2. `ssa` is filled with "[%arg0, %arg1]".
-///
-/// Indices can be scalable. For example, "4" in "[2, [4], 8]" is scalable.
-/// This notation is similar to how scalable dims are marked when defining
-/// Vectors. For each value in `integers`, the corresponding `bool` in
-/// `scalableVals` encodes whether it's a scalable index.
+/// type `I64ArrayAttr`. Parse a mixed list where each element is either a
+/// static integer or an SSA value. Fill `integers` with the integer ArrayAttr,
+/// where `kDynamic` encodes the position of SSA values. Add the parsed SSA
+/// values to `values` in-order.
+///
+/// If `valueTypes` is provided, fill it with the types corresponding to each
+/// value in `values`. Otherwise, the caller must handle the types.
+///
+/// Integer indices can also be scalable, denoted by the square bracket (e.g.,
+/// "[2, [4], 8]"). For each value in `integers`, the corresponding `bool` in
+/// `scalables` encodes whether it's a scalable index.
+///
+/// Examples:
+///
+/// * After parsing "[%arg0 : index, 7, 42, %arg42 : i32]":
+/// 1. `result` is filled with `[kDynamic, 7, 42, kDynamic]`
+/// 2. `values` is filled with "[%arg0, %arg1]".
+/// 3. `scalables` is filled with `[false, true, false]`.
+///
+/// * After parsing `[2, [4], 8]`:
+/// 1. `result` is filled with `[2, 4, 8]`
+/// 2. `values` is empty.
+/// 3. `scalables` is filled with `[false, true, false]`.
+///
ParseResult parseDynamicIndexList(
OpAsmParser &parser,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &values,
- DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalableVals,
+ DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalables,
SmallVectorImpl<Type> *valueTypes = nullptr,
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square);
inline ParseResult parseDynamicIndexList(
@@ -149,9 +180,9 @@ inline ParseResult parseDynamicIndexList(
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &values,
DenseI64ArrayAttr &integers, SmallVectorImpl<Type> *valueTypes = nullptr,
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square) {
- DenseBoolArrayAttr scalableVals = {};
- return parseDynamicIndexList(parser, values, integers, scalableVals,
- valueTypes, delimiter);
+ DenseBoolArrayAttr scalables;
+ return parseDynamicIndexList(parser, values, integers, scalables, valueTypes,
+ delimiter);
}
/// Verify that a the `values` has as many elements as the number of entries in
>From 74cb8a7a67023bfb4236ee8e3a88a5aad05ff776 Mon Sep 17 00:00:00 2001
From: Diego Caballero <dieg0ca6aller0 at gmail.com>
Date: Mon, 13 Jan 2025 10:36:15 -0800
Subject: [PATCH 2/2] Feedback
---
.../mlir/Interfaces/ViewLikeInterface.h | 49 +++++++++++--------
mlir/lib/Interfaces/ViewLikeInterface.cpp | 11 +++--
2 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/mlir/include/mlir/Interfaces/ViewLikeInterface.h b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
index 07c6465063dd30..8f07e43f847ae5 100644
--- a/mlir/include/mlir/Interfaces/ViewLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/ViewLikeInterface.h
@@ -98,13 +98,17 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
///
/// If `valueTypes` is provided, the corresponding type of each dynamic value is
/// printed. Otherwise, the type is not printed. Each type must match the type
-/// of the corresponding value in `values`. The type for integer elements is
-/// `i64` by default and never printed.
-///
-/// Integer indices can also be scalable, denoted with square brackets (e.g.,
-/// "[2, [4], 8]"). For each value in `integers`, the corresponding `bool` in
-/// `scalables` encodes whether it's a scalable index. If `scalables` is empty
-/// then assume that all indices are non-scalable.
+/// of the corresponding value in `values`. `valueTypes` is redundant for
+/// printing as we can retrieve the types from the actual `values`. However,
+/// `valueTypes` is needed for parsing and we must keep the API symmetric for
+/// parsing and printing. The type for integer elements is `i64` by default and
+/// never printed.
+///
+/// Integer indices can also be scalable in the context of scalable vectors,
+/// denoted by square brackets (e.g., "[2, [4], 8]"). For each value in
+/// `integers`, the corresponding `bool` in `scalableFlags` encodes whether it's
+/// a scalable index. If `scalableFlags` is empty then assume that all indices
+/// are non-scalable.
///
/// Examples:
///
@@ -122,21 +126,21 @@ class OpWithOffsetSizesAndStridesConstantArgumentFolder final
///
/// * Input: `integers = [2, 4, 8]`,
/// `values = []` and
-/// `scalables = [false, true, false]`
+/// `scalableFlags = [false, true, false]`
/// prints:
/// `[2, [4], 8]`
///
void printDynamicIndexList(
OpAsmPrinter &printer, Operation *op, OperandRange values,
- ArrayRef<int64_t> integers, ArrayRef<bool> scalables,
+ ArrayRef<int64_t> integers, ArrayRef<bool> scalableFlags,
TypeRange valueTypes = TypeRange(),
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square);
inline void printDynamicIndexList(
OpAsmPrinter &printer, Operation *op, OperandRange values,
ArrayRef<int64_t> integers, TypeRange valueTypes = TypeRange(),
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square) {
- return printDynamicIndexList(printer, op, values, integers, /*scalables=*/{},
- valueTypes, delimiter);
+ return printDynamicIndexList(printer, op, values, integers,
+ /*scalableFlags=*/{}, valueTypes, delimiter);
}
/// Parser hooks for custom directive in assemblyFormat.
@@ -151,28 +155,31 @@ inline void printDynamicIndexList(
/// values to `values` in-order.
///
/// If `valueTypes` is provided, fill it with the types corresponding to each
-/// value in `values`. Otherwise, the caller must handle the types.
+/// value in `values`. Otherwise, the caller must handle the types and parsing
+/// will fail if the type of the value is found (e.g., `[%arg0 : index, 3, %arg1
+/// : index]`).
///
-/// Integer indices can also be scalable, denoted by the square bracket (e.g.,
-/// "[2, [4], 8]"). For each value in `integers`, the corresponding `bool` in
-/// `scalables` encodes whether it's a scalable index.
+/// Integer indices can also be scalable in the context of scalable vectors,
+/// denoted by square brackets (e.g., "[2, [4], 8]"). For each value in
+/// `integers`, the corresponding `bool` in `scalableFlags` encodes whether it's
+/// a scalable index.
///
/// Examples:
///
/// * After parsing "[%arg0 : index, 7, 42, %arg42 : i32]":
/// 1. `result` is filled with `[kDynamic, 7, 42, kDynamic]`
/// 2. `values` is filled with "[%arg0, %arg1]".
-/// 3. `scalables` is filled with `[false, true, false]`.
+/// 3. `scalableFlags` is filled with `[false, true, false]`.
///
/// * After parsing `[2, [4], 8]`:
/// 1. `result` is filled with `[2, 4, 8]`
/// 2. `values` is empty.
-/// 3. `scalables` is filled with `[false, true, false]`.
+/// 3. `scalableFlags` is filled with `[false, true, false]`.
///
ParseResult parseDynamicIndexList(
OpAsmParser &parser,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &values,
- DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalables,
+ DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalableFlags,
SmallVectorImpl<Type> *valueTypes = nullptr,
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square);
inline ParseResult parseDynamicIndexList(
@@ -180,9 +187,9 @@ inline ParseResult parseDynamicIndexList(
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &values,
DenseI64ArrayAttr &integers, SmallVectorImpl<Type> *valueTypes = nullptr,
AsmParser::Delimiter delimiter = AsmParser::Delimiter::Square) {
- DenseBoolArrayAttr scalables;
- return parseDynamicIndexList(parser, values, integers, scalables, valueTypes,
- delimiter);
+ DenseBoolArrayAttr scalableFlags;
+ return parseDynamicIndexList(parser, values, integers, scalableFlags,
+ valueTypes, delimiter);
}
/// Verify that a the `values` has as many elements as the number of entries in
diff --git a/mlir/lib/Interfaces/ViewLikeInterface.cpp b/mlir/lib/Interfaces/ViewLikeInterface.cpp
index ca33636336bf0c..57b5cce7bb13b0 100644
--- a/mlir/lib/Interfaces/ViewLikeInterface.cpp
+++ b/mlir/lib/Interfaces/ViewLikeInterface.cpp
@@ -113,7 +113,8 @@ static char getRightDelimiter(AsmParser::Delimiter delimiter) {
void mlir::printDynamicIndexList(OpAsmPrinter &printer, Operation *op,
OperandRange values,
ArrayRef<int64_t> integers,
- ArrayRef<bool> scalables, TypeRange valueTypes,
+ ArrayRef<bool> scalableFlags,
+ TypeRange valueTypes,
AsmParser::Delimiter delimiter) {
char leftDelimiter = getLeftDelimiter(delimiter);
char rightDelimiter = getRightDelimiter(delimiter);
@@ -126,7 +127,7 @@ void mlir::printDynamicIndexList(OpAsmPrinter &printer, Operation *op,
unsigned dynamicValIdx = 0;
unsigned scalableIndexIdx = 0;
llvm::interleaveComma(integers, printer, [&](int64_t integer) {
- if (!scalables.empty() && scalables[scalableIndexIdx])
+ if (!scalableFlags.empty() && scalableFlags[scalableIndexIdx])
printer << "[";
if (ShapedType::isDynamic(integer)) {
printer << values[dynamicValIdx];
@@ -136,7 +137,7 @@ void mlir::printDynamicIndexList(OpAsmPrinter &printer, Operation *op,
} else {
printer << integer;
}
- if (!scalables.empty() && scalables[scalableIndexIdx])
+ if (!scalableFlags.empty() && scalableFlags[scalableIndexIdx])
printer << "]";
scalableIndexIdx++;
@@ -148,7 +149,7 @@ void mlir::printDynamicIndexList(OpAsmPrinter &printer, Operation *op,
ParseResult mlir::parseDynamicIndexList(
OpAsmParser &parser,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &values,
- DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalables,
+ DenseI64ArrayAttr &integers, DenseBoolArrayAttr &scalableFlags,
SmallVectorImpl<Type> *valueTypes, AsmParser::Delimiter delimiter) {
SmallVector<int64_t, 4> integerVals;
@@ -183,7 +184,7 @@ ParseResult mlir::parseDynamicIndexList(
return parser.emitError(parser.getNameLoc())
<< "expected SSA value or integer";
integers = parser.getBuilder().getDenseI64ArrayAttr(integerVals);
- scalables = parser.getBuilder().getDenseBoolArrayAttr(scalableVals);
+ scalableFlags = parser.getBuilder().getDenseBoolArrayAttr(scalableVals);
return success();
}
More information about the Mlir-commits
mailing list