[PATCH] D77787: [MLIR] Introduce a trait that defines a new scope for auto allocation

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 9 11:45:09 PDT 2020


rriddle added inline comments.


================
Comment at: mlir/include/mlir/IR/OpDefinition.h:1039
+  static LogicalResult verifyTrait(Operation *op) {
+    if (op->getNumRegions() == 0)
+      return op->emitOpError("is expected to have regions");
----------------
bondhugula wrote:
> rriddle wrote:
> > bondhugula wrote:
> > > rriddle wrote:
> > > > What happens if the operation has a variadic number of regions?
> > > That should be fine. The allocations of each are still to be freed when control transfers from that respective region to the op. 
> > Can we remove this check then? Otherwise, specific forms of operations would be legal and others not.
> Ops that always have zero regions can't meaningfully have scoped allocations. We could just allow it for VariadicRegions instead by basing this check on the ZeroRegion trait?
I was wondering about the situations where we may end up in such a case, e.g. a switch-like dispatch operation that ends up having no cases(either by optimization, or generated by the front end). I would just rather avoid the situation that an optimization pass ends up generating a verifier failure for otherwise valid IR. Debugging that seems like it would be a pain.

ZeroRegion trait SGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77787/new/

https://reviews.llvm.org/D77787





More information about the llvm-commits mailing list