You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[mlir] [irdl] Add support for regions in irdl-to-cpp (llvm#158540)
Fixesllvm#158034
For the input
```mlir
irdl.dialect @conditional_dialect {
// A conditional operation with regions
irdl.operation @conditional {
// Create region constraints
%r0 = irdl.region // Unconstrained region
%r1 = irdl.region() // Region with no entry block arguments
%v0 = irdl.any
%r2 = irdl.region(%v0) // Region with one i1 entry block argument
irdl.regions(cond: %r2, then: %r0, else: %r1)
}
}
```
This produces the following cpp:
https://gist.github.com/j2kun/d2095f108efbd8d403576d5c460e0c00
Summary of changes:
- The op class and adaptor get named accessors to the regions `Region
&get<RegionName>()` and `getRegions()`
- The op now gets `OpTrait::NRegions<3>` and `OpInvariants` to trigger
the region verification
- Support for region block argument constraints is added, but not
working for all constraints until codegen for `irdl.is` is added (filed
llvm#161018 and left a TODO).
- Helper functions for the individual verification steps are added,
following mlir-tblgen's format (in the above gist,
`__mlir_irdl_local_region_constraint_ConditionalOp_cond` and similar),
and `verifyInvariantsImpl` that calls them.
- Regions are added in the builder
## Questions for the reviewer
### What is the "correct" interface for verification?
I used `mlir-tblgen` on an analogous version of the example
`ConditionalOp` in this PR, and I see an `::mlir::OpTrait::OpInvariants`
trait as well as
```cpp
::llvm::LogicalResult ConditionalOp::verifyInvariantsImpl() {
{
unsigned index = 0; (void)index;
for (auto ®ion : ::llvm::MutableArrayRef((*this)->getRegion(0)))
if (::mlir::failed(__mlir_ods_local_region_constraint_test1(*this, region, "cond", index++)))
return ::mlir::failure();
for (auto ®ion : ::llvm::MutableArrayRef((*this)->getRegion(1)))
if (::mlir::failed(__mlir_ods_local_region_constraint_test1(*this, region, "then", index++)))
return ::mlir::failure();
for (auto ®ion : ::llvm::MutableArrayRef((*this)->getRegion(2)))
if (::mlir::failed(__mlir_ods_local_region_constraint_test1(*this, region, "else", index++)))
return ::mlir::failure();
}
return ::mlir::success();
}
::llvm::LogicalResult ConditionalOp::verifyInvariants() {
if(::mlir::succeeded(verifyInvariantsImpl()) && ::mlir::succeeded(verify()))
return ::mlir::success();
return ::mlir::failure();
}
```
However, `OpInvariants` only seems to need `verifyInvariantsImpl`, so
it's not clear to me what is the purpose of the `verifyInvariants`
function, or, if I leave out `verifyInvariants`, whether I need to call
`verify()` in my implementation of `verifyInvariantsImpl`. In this PR, I
omitted `verifyInvariants` and generated `verifyInvariantsImpl`.
### Is testing sufficient?
I am not certain I implemented the builders properly, and it's unclear
to me to what extent the existing tests check this (which look like they
compile the generated cpp, but don't actually use it). Did I omit some
standard function or overload?
---------
Co-authored-by: Jeremy Kun <[email protected]>
0 commit comments