[MLIR][IRDL] Support camelCase segment size attributes in IRDL verifier #168836
+36
−36
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-mlir-irdl Author: Twice (PragmaTwice) ChangesIn 2 years ago, However, the op verifiers in IRDL loading phase is still using old attributes like This PR is to support to use camelCase segment size attributes in IRDL verifier. Note that to avoid breaking change Full diff: https://github.com/llvm/llvm-project/pull/168836.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/IRDL/IRDLLoading.cpp b/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
index 212ccc931f6e0..22b91a844fa8d 100644
--- a/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
+++ b/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
@@ -169,8 +169,11 @@ LogicalResult getSegmentSizes(Operation *op, StringRef elemName,
LogicalResult getOperandSegmentSizes(Operation *op,
ArrayRef<Variadicity> variadicities,
SmallVectorImpl<int> &segmentSizes) {
- return getSegmentSizes(op, "operand", "operand_segment_sizes",
- op->getNumOperands(), variadicities, segmentSizes);
+ StringRef attrName = op->getAttr("operandSegmentSizes")
+ ? "operandSegmentSizes"
+ : "operand_segment_sizes";
+ return getSegmentSizes(op, "operand", attrName, op->getNumOperands(),
+ variadicities, segmentSizes);
}
/// Compute the segment sizes of the given results.
@@ -180,8 +183,11 @@ LogicalResult getOperandSegmentSizes(Operation *op,
LogicalResult getResultSegmentSizes(Operation *op,
ArrayRef<Variadicity> variadicities,
SmallVectorImpl<int> &segmentSizes) {
- return getSegmentSizes(op, "result", "result_segment_sizes",
- op->getNumResults(), variadicities, segmentSizes);
+ StringRef attrName = op->getAttr("resultSegmentSizes")
+ ? "resultSegmentSizes"
+ : "result_segment_sizes";
+ return getSegmentSizes(op, "result", attrName, op->getNumResults(),
+ variadicities, segmentSizes);
}
/// Verify that the given operation satisfies the given constraints.
diff --git a/mlir/test/Dialect/IRDL/variadics.mlir b/mlir/test/Dialect/IRDL/variadics.mlir
index a8871fcf5ebd9..97b28e0998e00 100644
--- a/mlir/test/Dialect/IRDL/variadics.mlir
+++ b/mlir/test/Dialect/IRDL/variadics.mlir
@@ -161,6 +161,18 @@ func.func @testMultOperands(%x: i16, %y: i32, %z: i64) {
// CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}) {operand_segment_sizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
return
}
+// -----
+
+// Check that attribute 'operandSegmentSizes' can work well as 'operand_segment_sizes'
+func.func @testMultOperands(%x: i16, %y: i32, %z: i64) {
+ "testvar.var_and_opt_operand"(%x, %x, %z) {operandSegmentSizes = array<i32: 2, 0, 1>} : (i16, i16, i64) -> ()
+ // CHECK: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 2, 0, 1>} : (i16, i16, i64) -> ()
+ "testvar.var_and_opt_operand"(%x, %x, %y, %z) {operandSegmentSizes = array<i32: 2, 1, 1>} : (i16, i16, i32, i64) -> ()
+ // CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 2, 1, 1>} : (i16, i16, i32, i64) -> ()
+ "testvar.var_and_opt_operand"(%y, %z) {operandSegmentSizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
+ // CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
+ return
+}
// -----
@@ -365,6 +377,19 @@ func.func @testMultResults() {
// -----
+// Check that attribute 'resultSegmentSizes' can work well as 'result_segment_sizes'
+func.func @testMultResults() {
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 0, 1>} : () -> (i16, i16, i64)
+ // CHECK: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 0, 1>} : () -> (i16, i16, i64)
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 1, 1>} : () -> (i16, i16, i32, i64)
+ // CHECK-NEXT: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 1, 1>} : () -> (i16, i16, i32, i64)
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 0, 1, 1>} : () -> (i32, i64)
+ // CHECK-NEXT: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 0, 1, 1>} : () -> (i32, i64)
+ return
+}
+
+// -----
+
// Check that the segment sizes expects non-negative values
func.func @testMultResultsSegmentNegative() {
// expected-error@+1 {{'result_segment_sizes' attribute for specifying result segments must have non-negative values}}
|
Member
|
@llvm/pr-subscribers-mlir Author: Twice (PragmaTwice) ChangesIn 2 years ago, However, the op verifiers in IRDL loading phase is still using old attributes like This PR is to support to use camelCase segment size attributes in IRDL verifier. Note that to avoid breaking change Full diff: https://github.com/llvm/llvm-project/pull/168836.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/IRDL/IRDLLoading.cpp b/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
index 212ccc931f6e0..22b91a844fa8d 100644
--- a/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
+++ b/mlir/lib/Dialect/IRDL/IRDLLoading.cpp
@@ -169,8 +169,11 @@ LogicalResult getSegmentSizes(Operation *op, StringRef elemName,
LogicalResult getOperandSegmentSizes(Operation *op,
ArrayRef<Variadicity> variadicities,
SmallVectorImpl<int> &segmentSizes) {
- return getSegmentSizes(op, "operand", "operand_segment_sizes",
- op->getNumOperands(), variadicities, segmentSizes);
+ StringRef attrName = op->getAttr("operandSegmentSizes")
+ ? "operandSegmentSizes"
+ : "operand_segment_sizes";
+ return getSegmentSizes(op, "operand", attrName, op->getNumOperands(),
+ variadicities, segmentSizes);
}
/// Compute the segment sizes of the given results.
@@ -180,8 +183,11 @@ LogicalResult getOperandSegmentSizes(Operation *op,
LogicalResult getResultSegmentSizes(Operation *op,
ArrayRef<Variadicity> variadicities,
SmallVectorImpl<int> &segmentSizes) {
- return getSegmentSizes(op, "result", "result_segment_sizes",
- op->getNumResults(), variadicities, segmentSizes);
+ StringRef attrName = op->getAttr("resultSegmentSizes")
+ ? "resultSegmentSizes"
+ : "result_segment_sizes";
+ return getSegmentSizes(op, "result", attrName, op->getNumResults(),
+ variadicities, segmentSizes);
}
/// Verify that the given operation satisfies the given constraints.
diff --git a/mlir/test/Dialect/IRDL/variadics.mlir b/mlir/test/Dialect/IRDL/variadics.mlir
index a8871fcf5ebd9..97b28e0998e00 100644
--- a/mlir/test/Dialect/IRDL/variadics.mlir
+++ b/mlir/test/Dialect/IRDL/variadics.mlir
@@ -161,6 +161,18 @@ func.func @testMultOperands(%x: i16, %y: i32, %z: i64) {
// CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}) {operand_segment_sizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
return
}
+// -----
+
+// Check that attribute 'operandSegmentSizes' can work well as 'operand_segment_sizes'
+func.func @testMultOperands(%x: i16, %y: i32, %z: i64) {
+ "testvar.var_and_opt_operand"(%x, %x, %z) {operandSegmentSizes = array<i32: 2, 0, 1>} : (i16, i16, i64) -> ()
+ // CHECK: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 2, 0, 1>} : (i16, i16, i64) -> ()
+ "testvar.var_and_opt_operand"(%x, %x, %y, %z) {operandSegmentSizes = array<i32: 2, 1, 1>} : (i16, i16, i32, i64) -> ()
+ // CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}, %{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 2, 1, 1>} : (i16, i16, i32, i64) -> ()
+ "testvar.var_and_opt_operand"(%y, %z) {operandSegmentSizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
+ // CHECK-NEXT: "testvar.var_and_opt_operand"(%{{.*}}, %{{.*}}) {operandSegmentSizes = array<i32: 0, 1, 1>} : (i32, i64) -> ()
+ return
+}
// -----
@@ -365,6 +377,19 @@ func.func @testMultResults() {
// -----
+// Check that attribute 'resultSegmentSizes' can work well as 'result_segment_sizes'
+func.func @testMultResults() {
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 0, 1>} : () -> (i16, i16, i64)
+ // CHECK: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 0, 1>} : () -> (i16, i16, i64)
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 1, 1>} : () -> (i16, i16, i32, i64)
+ // CHECK-NEXT: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 2, 1, 1>} : () -> (i16, i16, i32, i64)
+ "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 0, 1, 1>} : () -> (i32, i64)
+ // CHECK-NEXT: "testvar.var_and_opt_result"() {resultSegmentSizes = array<i32: 0, 1, 1>} : () -> (i32, i64)
+ return
+}
+
+// -----
+
// Check that the segment sizes expects non-negative values
func.func @testMultResultsSegmentNegative() {
// expected-error@+1 {{'result_segment_sizes' attribute for specifying result segments must have non-negative values}}
|
🐧 Linux x64 Test Results
|
math-fehr
approved these changes
Nov 24, 2025
Contributor
math-fehr
left a comment
math-fehr
left a comment
There was a problem hiding this comment.
Thanks! Is it still worth it to support operand_segment_sizes at that point, since now everything should be moved to camelCase?
Member
Author
Ahh seems not. I have checked xdsl and found that it already did this replacement via xdslproject/xdsl@ca21d8e. I think we can drop the support to I'll make this change soon. |
Member
Author
Done in 895aa68. |
math-fehr
approved these changes
Nov 24, 2025
Contributor
math-fehr
left a comment
math-fehr
left a comment
There was a problem hiding this comment.
Perfect, thanks a lot!
Member
Author
|
Thank you for your review. I'll merge it soon. |
aadeshps-mcw
pushed a commit
to aadeshps-mcw/llvm-project
that referenced
this pull request
Nov 26, 2025
…er (llvm#168836) Two years ago, `operand_segment_sizes` and `result_segment_sizes` were renamed to `operandSegmentSizes` and `resultSegmentSizes` (check related commits, e.g. llvm@363b655). However, the op verifiers in IRDL loading phase is still using old attributes like `operand_segment_sizes` and `result_segment_sizes`, which causes some conflict, e.g. it is not compatible with the OpView builder in MLIR python bindings (which generates camelCase segment attributes). This PR is to support to use camelCase segment size attributes in IRDL verifier. Note that support of `operand_segment_sizes` and `result_segment_sizes` is dropped. I found this issue since I'm working on a new IRDL wrapper in the MLIR python bindings.
Priyanshu3820
pushed a commit
to Priyanshu3820/llvm-project
that referenced
this pull request
Nov 26, 2025
…er (llvm#168836) Two years ago, `operand_segment_sizes` and `result_segment_sizes` were renamed to `operandSegmentSizes` and `resultSegmentSizes` (check related commits, e.g. llvm@363b655). However, the op verifiers in IRDL loading phase is still using old attributes like `operand_segment_sizes` and `result_segment_sizes`, which causes some conflict, e.g. it is not compatible with the OpView builder in MLIR python bindings (which generates camelCase segment attributes). This PR is to support to use camelCase segment size attributes in IRDL verifier. Note that support of `operand_segment_sizes` and `result_segment_sizes` is dropped. I found this issue since I'm working on a new IRDL wrapper in the MLIR python bindings.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two years ago,
operand_segment_sizesandresult_segment_sizeswere renamed tooperandSegmentSizesandresultSegmentSizes(check related commits, e.g. 363b655).However, the op verifiers in IRDL loading phase is still using old attributes like
operand_segment_sizesandresult_segment_sizes, which causes some conflict, e.g. it is not compatible with the OpView builder in MLIR python bindings (which generates camelCase segment attributes).This PR is to support to use camelCase segment size attributes in IRDL verifier. Note that support of
operand_segment_sizesandresult_segment_sizesis dropped.I found this issue since I'm working on a new IRDL wrapper in the MLIR python bindings.