This will later be used during canonicalization steps to replace
statically known passing constraints. This might later be removed once
witnesses can be represented as Attributes.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM, thanks :)
See my comment in the other patch about using this in ShapeDialect::materializeConstant
(I'm on the fence about having a const_witness {true,false} vs true_witness/false_witness ops; that latter is a bit easier to match with isa<TrueWitnessOp> so I tend to lean towards it, but the former is more consistent with const_shape/const_size and maybe a simple helper can recuperate the advantages of isa<TrueWitnessOp>'s simplicity; feel free to choose whatever makes sense)
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td | ||
---|---|---|
502 | I'd be +1 this for consistency. If we have a witness attribute then I think we could just use std.const too. (then little helper function to check isTrueWitness(Operation* op), which we could add already) | |
506 | Summaries are single line sentence fragments that don't end with punctuation (https://mlir.llvm.org/docs/OpDefinitions/#operation-documentation). I see we are a bit inconsistent in this file on that already. Might just submit a quick NFC change resolving that, | |
508 | Should we perhaps start with some lead up before the example? And put the MLIR in a code block? (I added syntax highlighting especially for this :-)) This operation produces a constant true witness value. This often arises during constant folding/canonicalization, e.g., mlir ... Slightly repetitive with the summary, but given custom asm syntax these will be split in the generated docs. |
Thanks!
mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td | ||
---|---|---|
502 | I like the isTrueWitness to hide how this is implemented. Then we can move this to ConstantLike later without changing users. |
I'd be +1 this for consistency. If we have a witness attribute then I think we could just use std.const too.
(then little helper function to check isTrueWitness(Operation* op), which we could add already)