[PATCH] D80307: [mlir] Add a shape op that always returns a successful witness
Jacques Pienaar via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 20 17:40:53 PDT 2020
jpienaar accepted this revision.
jpienaar marked an inline comment as done.
jpienaar added inline comments.
This revision is now accepted and ready to land.
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:502
+// TODO: Support witness attributes and then make this ConstantLike.
+// Note: This operation might be replaced with a general op that takes a
----------------
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)
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:506
+def Shape_TrueWitnessOp : Shape_Op<"true_witness", [NoSideEffect]> {
+ let summary = "An operation that returns a successful witness.";
+ let description = [{
----------------
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,
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:508
+ let description = [{
+ %0 = shape.const_shape [1, 2, 3]
+ %1 = shape.const_shape [1, 2, 3]
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80307/new/
https://reviews.llvm.org/D80307
More information about the llvm-commits
mailing list