[PATCH] D79717: [MLIR] Add shape.witness type and ops
Sean Silva via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 12 13:59:56 PDT 2020
silvas accepted this revision.
silvas added inline comments.
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:24
+ let typeDescription = [{
+ A witness is a token to ordering of code using relying on information
+ obtained from passing assertions. Witnesses do not represent any physical
----------------
typo "using relying"
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:25
+ A witness is a token to ordering of code using relying on information
+ obtained from passing assertions. Witnesses do not represent any physical
+ data, so they do not exist at execution time.
----------------
"They are only used as a structural device in the compiler to maintain ordering."
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:32
+ "assuming_" operations will take witnesses as input and represent only
+ information to the compiler, so they do not exist in executing code. Cod
+ that is dependent on "assuming_" operations can assume all cstr operations
----------------
typo Cod
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:397
+
+def Shape_AssumeAllOp : Shape_Op<"assuming_all", []> {
+ let summary = "Return a logical AND of all witnesses.";
----------------
def name inconsistent with op name.
================
Comment at: mlir/include/mlir/Dialect/Shape/IR/ShapeOps.td:434
+
+def Shape_ExecAssuming : Shape_Op<"assuming_region",
+ [SingleBlockImplicitTerminator<"AssumingRegionYield">,
----------------
def name is inconsistent with op name.
================
Comment at: mlir/lib/Dialect/Shape/IR/Shape.cpp:294
+//===----------------------------------------------------------------------===//
+LogicalResult
+AnyOp::inferReturnTypes(MLIRContext *context, Optional<Location> location,
----------------
nit: newline after block comment
================
Comment at: mlir/test/Dialect/Shape/ops.mlir:71
+ %w1 = "shape.cstr_eq"(%0, %1) : (!shape.shape, !shape.shape) -> !shape.witness
+ %2 = "shape.any"(%0, %1) : (!shape.shape, !shape.shape) -> !shape.shape
+ %w3 = "shape.all"(%w0, %w1) : (!shape.witness, !shape.witness) -> !shape.witness
----------------
Let's move the shape.any into the region?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79717/new/
https://reviews.llvm.org/D79717
More information about the llvm-commits
mailing list