[PATCH] D80307: [mlir] Add a shape op that always returns a successful witness

[Jacques Pienaar via Phabricator via llvm-commits]

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