dialects: (irdl) Update for mlir compatibility #3002
Conversation
alexarice
changed the title
alexarice
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3002 +/- ##
=======================================
Coverage 89.85% 89.86%
=======================================
Files 413 413
Lines 51862 51928 +66
Branches 8009 8028 +19
=======================================
+ Hits 46600 46663 +63
- Misses 3972 3975 +3
Partials 1290 1290 ☔ View full report in Codecov by Sentry. |
xdsl/dialects/irdl/pyrdl_to_irdl.py
Outdated
| @@ -51,7 +51,7 @@ def op_def_to_irdl(op: type[IRDLOperation]) -> OperationOp: | |||
| if result_values: | |||
| builder.insert(ResultsOp(result_values)) | |||
|
|
|||
| return OperationOp(op_def.name, Region([block])) | |||
| return OperationOp(op_def.name.split(".")[-1], Region([block])) | |||
There was a problem hiding this comment.
this doesn't quite feel right, are there any tests for this? My understanding is the dialect name is everything up to the first dot, and the op name is everything after it
There was a problem hiding this comment.
We probably want to centralise this logic somewhere
There was a problem hiding this comment.
Yep, should be the other way around! Split on the first dot and keep whatever's after.
More concretely, some ops have nested-like names, say scf.forall.in_parallel : dialect still is just scf, op name is forall.in_parallel, dot including.
And yes, we probably want to centraluze it ahah
There was a problem hiding this comment.
Any idea where the centralisation should live?
There was a problem hiding this comment.
Now rebased onto #3003
xdsl/dialects/irdl/pyrdl_to_irdl.py
Outdated
| @@ -51,7 +51,7 @@ def op_def_to_irdl(op: type[IRDLOperation]) -> OperationOp: | |||
| if result_values: | |||
| builder.insert(ResultsOp(result_values)) | |||
|
|
|||
| return OperationOp(op_def.name, Region([block])) | |||
| return OperationOp(op_def.name.split(".")[-1], Region([block])) | |||
There was a problem hiding this comment.
Yep, should be the other way around! Split on the first dot and keep whatever's after.
More concretely, some ops have nested-like names, say scf.forall.in_parallel : dialect still is just scf, op name is forall.in_parallel, dot including.
And yes, we probably want to centraluze it ahah
alexarice
changed the title
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
Co-authored-by: Sasha Lopoukhine <superlopuh@gmail.com>
alexarice
force-pushed
the
alexarice/irdl-update
branch
from
5c5cff4 to
00a7acb
Compare
| class VariadicityAttr(EnumAttribute[VariadicityEnum], SpacedOpaqueSyntaxAttribute): | ||
| name = "irdl.variadicity" |
There was a problem hiding this comment.
| class VariadicityAttr(EnumAttribute[VariadicityEnum], SpacedOpaqueSyntaxAttribute): | |
| name = "irdl.variadicity" | |
| class VariadicityAttr(EnumAttribute[VariadicityEnum], SpacedOpaqueSyntaxAttribute): | |
| name = "irdl.variadicity" | |
| SINGLE = VariadicityAttr(VariadicityEnum.SINGLE) |
etc
Added the
irdl.c_predoperation as well as adding variadicity attributes to operands and results. Also updated the pyrdl-to-irdl script so that it prints symbols correctly.