[PATCH] D78863: [MLIR] Introduce op trait PolyhedralScope

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/include/mlir/IR/OpDefinition.h:1056
+/// operation. For more details, see `Traits.md#PolyhedralScope`.
+template <typename ConcreteType>
+class PolyhedralScope : public TraitBase<ConcreteType, PolyhedralScope> {
----------------
Is this trait applicable to anything outside of the affine dialect? If not I don't really see why it should be here and not in the affine dialect instead.


================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:104
   if (auto arg = value.dyn_cast<BlockArgument>())
-    return isFunctionRegion(arg.getOwner()->getParent());
-  return isFunctionRegion(value.getDefiningOp()->getParentRegion());
+    return arg.getOwner()->getParentOp() == region->getParentOp();
+  return value.getDefiningOp()->getParentOp() == region->getParentOp();
----------------
`value.getParentRegion() == region`?


================
Comment at: mlir/lib/Dialect/Affine/IR/AffineOps.cpp:249
+  // Constant operation is ok.
+  if (isa<ConstantOp>(defOp))
+    return true;
----------------
Can we use m_Constant instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78863