Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MLIR verifier to BConv op #406

Open
lgeiger opened this issue Jun 15, 2020 · 0 comments
Open

Add MLIR verifier to BConv op #406

lgeiger opened this issue Jun 15, 2020 · 0 comments
Labels
good first issue Good for newcomers internal-improvement Internal Improvements and Maintenance

Comments

@lgeiger
Copy link
Member

lgeiger commented Jun 15, 2020

Since #384 we can make use of the IR verification features of MLIR.

Our BConv op has a few parameter combinations where it can throw during Init or Prepare. In the converter we try to make sure that none of those cases will ever occur, but it would be good to add verification code so that potential errors are already caught during conversion and not at runtime which makes for a much nicer user experience if something goes wrong (especially when relying one some of our experimental features).

Most of the verification is already autogenerated by MLIR based on the attr constraints, but we also have some unsupported cases that depend on the interaction between different attributes [e.g. padding and fused activation functions).
This verification can be done nicely by populating the following verifier boilerplate with the relevant constraints of our TFLite kernel:

diff --git a/larq_compute_engine/mlir/ir/lce_ops.cc b/larq_compute_engine/mlir/ir/lce_ops.cc
index 4283888..9ec481a 100644
--- a/larq_compute_engine/mlir/ir/lce_ops.cc
+++ b/larq_compute_engine/mlir/ir/lce_ops.cc
@@ -47,5 +47,11 @@ LarqDialect::LarqDialect(MLIRContext* context)
 #include "larq_compute_engine/mlir/ir/lce_ops.cc.inc"
       >();
 }
+
+LogicalResult Verify(Bconv2dOp op) {
+  return mlir::success();
+  return op.emitOpError("Something is not correct!");
+}
+
 }  // namespace TF
 }  // namespace mlir
diff --git a/larq_compute_engine/mlir/ir/lce_ops.td b/larq_compute_engine/mlir/ir/lce_ops.td
index d45ef14..a5c54ec 100644
--- a/larq_compute_engine/mlir/ir/lce_ops.td
+++ b/larq_compute_engine/mlir/ir/lce_ops.td
@@ -111,6 +111,8 @@ TODO
   let results = (outs
     TensorOf<[F32, I32, QI8]>:$output
   );
+
+  /* let verifier = [{ return Verify(*this); }]; */
 }

 def LQ_BMaxPool2dOp : LQ_Op<"BMaxPool2d", [NoSideEffect]> {

I am labeling this as a "good first issue" since it is a well scoped problem and we are happy to assist if anyone wants to tackle this.

@lgeiger lgeiger added good first issue Good for newcomers internal-improvement Internal Improvements and Maintenance labels Jun 15, 2020
@lgeiger lgeiger added this to the 0.4 milestone Jul 21, 2020
@lgeiger lgeiger removed this from the 0.4 milestone Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers internal-improvement Internal Improvements and Maintenance
Projects
None yet
Development

No branches or pull requests

1 participant