- Change callback signature bool(Operation *) -> Optional<bool>(Operation *)
- addDynamicallyLegalOp add callback to the chain
- If callback returned empty Optional next callback in chain will be called
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Old discourse thread https://llvm.discourse.group/t/markunknownopdynamicallylegal-is-not-composable/3800
Initially I wanted to do this only for markUnknownOpDynamicallyLegal, but now I have usecase for addDynamicallyLegalOp too:
I have getitem op in my dialect which can be applied to arrays or tuples with different set of conversions and legality setup, and I want to setup them from different places.
mlir/include/mlir/Transforms/DialectConversion.h | ||
---|---|---|
664 | I think Optional<smth> approach is in line with other APIs (e.g. TypeConverter) and Optional<bool> allow to use existing bool callbacks as is |
I am still iffy about the Optional<bool>, but am fine with it for now for consistency sake.
mlir/lib/Transforms/Utils/DialectConversion.cpp | ||
---|---|---|
2712–2714 | nit: Spell out auto here, and also merge the variable into the if. | |
mlir/unittests/Transforms/DialectConversion.cpp | ||
19–24 | nit: Pull the function out of the anonymous namespace. |
I would consider Optional<bool> to be a bit of anti-pattern. Can we switch to an enum/Optional<enum> instead?