Enables using OpAdaptor's verifier as trait. This allows for reordering
checking of OpAdaptor verification. Currently this verify method is
emitted in verify method via ODS which results in it being invoked last.
The basic verification of that the OpAdaptor does would be more
beneficial earlier and could rather always be verified first (e.g.,
automatically prepend OpAdaptorVerifier to traits in ODS) but this
change merely adds the trait pending discussion on alternatives to
achieve the above (which is also why its added only inside TestOps.td).
Details
- Reviewers
mehdi_amini rriddle antiagainst bondhugula
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/include/mlir/IR/OpDefinition.h | ||
---|---|---|
294 | I think this is not placed at the right point in this file and is also missing the banner comment. Below, you have: "Operand traits", "Result traits", "Misc traits", etc. Could you please put them under one of those or introduce a new one. | |
294 | Could you please also expand the doc comment a bit to make it self-contained? Your commit summary actually has some information, but things aren't clear from this. |
Is there more planned here? Seems like uses of this trait right now will result in double verification + double code size?
Correct (double verification, I don't think double code size as both calls the same verify method, but the compiler could inline). Next step would be to have ODS generate this as its first trait rather than invoking it in verify method generated. Now, this will result in many errors getting reported in different order and needing to update tests. I wanted to have this added/discussed in isolation before updating the tests (this only affects the one test op redundantly for now). And then the checking could be removed from type infrence methods using the adaptor verify manually today too (so that type inference can use attributes that are supposed to be specified).
The adaptor generated is also not as complete as it should be (it doesn't verify regions yet, nor does it verify as much of the operands as it could). So I'd want that more complete too.
I think this is not placed at the right point in this file and is also missing the banner comment. Below, you have: "Operand traits", "Result traits", "Misc traits", etc. Could you please put them under one of those or introduce a new one.