diff options
author | Jeremy Kun <jkun@google.com> | 2025-10-02 10:56:40 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-10-02 10:56:40 -0700 |
commit | 99d85906c542c3801a24137ba6d6f2c367308563 (patch) | |
tree | 5f0b26fd9651bdf2fb2bccfdd5467464c5d2cfd4 /flang/unittests/Optimizer/FIRContextTest.cpp | |
parent | 67c000efb7bb7858f9fb6e577f2c9d1f24291ba0 (diff) | |
download | llvm-main.zip llvm-main.tar.gz llvm-main.tar.bz2 |
Fixes https://github.com/llvm/llvm-project/issues/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
https://github.com/llvm/llvm-project/issues/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 <j2kun@users.noreply.github.com>
Diffstat (limited to 'flang/unittests/Optimizer/FIRContextTest.cpp')
0 files changed, 0 insertions, 0 deletions