misc: raise error if variadic results types aren't referenced in assembly format #3416
Conversation
superlopuh
requested review from
alexarice,
math-fehr,
gabrielrodcanal and
PapyChacal
xdsl/dialects/experimental/air.py
Outdated
There was a problem hiding this comment.
@gabrielrodcanal
This seemed like the most natural change, since the init always specifies the result type anyway.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3416 +/- ##
=======================================
Coverage 90.16% 90.17%
=======================================
Files 455 455
Lines 57467 57482 +15
Branches 5532 5530 -2
=======================================
+ Hits 51815 51833 +18
+ Misses 4195 4193 -2
+ Partials 1457 1456 -1 ☔ View full report in Codecov by Sentry. |
…Error on definition
| @@ -154,7 +154,7 @@ class ChannelPutOp(IRDLOperation): | |||
| src_sizes = var_operand_def(IndexType()) | |||
| src_strides = var_operand_def(IndexType()) | |||
|
|
|||
| async_token = opt_result_def(AsyncTokenAttr()) | |||
| async_token = result_def(AsyncTokenAttr()) | |||
There was a problem hiding this comment.
Should these be in this PR?
There was a problem hiding this comment.
These tests fail with my changes. This is because we silently always set variadic (meaning also optional) results to have length 1. Our filecheck passes whether or not the results are present in the CHECK lines, because we match on substrings, so we silently ignore whether there are 0 or 1 results. Without the change on this line, the tests crash with the NotImplementedError I added in this PR.
Sort of a revert of #3416, but I believe this fixes the underlying problem. Changes range constraint inference so that passing in a length is optional. The rationale for this is that the length of operands is always known, but the length of results is never known (as they can always be omitted). Similarly updates the `can_infer` function.
…mbly format (xdslproject#3416) The number of results is not passed in when parsing operations. In the generic format, the type of the operation always specifies the types of the results, and `resultSegmentSizes` specifies the ranges of of the results if multiple are variadic. In order to support variadic results, the types an length of all variadic results must be present in the custom syntax.
Sort of a revert of xdslproject#3416, but I believe this fixes the underlying problem. Changes range constraint inference so that passing in a length is optional. The rationale for this is that the length of operands is always known, but the length of results is never known (as they can always be omitted). Similarly updates the `can_infer` function.
The number of results is not passed in when parsing operations. In the generic format, the type of the operation always specifies the types of the results, and
resultSegmentSizesspecifies the ranges of of the results if multiple are variadic. In order to support variadic results, the types an length of all variadic results must be present in the custom syntax.