[PATCH] D76413: [mlir][Linalg] NFC: Clean up for 0-D abstraction.

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 19 00:30:41 PDT 2020


rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/LinalgToLoops.cpp:90
+                                        ArrayRef<Value> outputBuffers) {
+  auto b = ScopedContext::getBuilder();
+  auto &block = op.region().front();
----------------
Isn't this returning a reference?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/LinalgToLoops.cpp:95
+  for (auto &op : block.without_terminator()) {
+    assert(op.getNumRegions() == 0);
+    auto *newOp = b.clone(op, map);
----------------
Can you add a message to this assert?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/LinalgToLoops.cpp:100
+
+  auto *yieldOp = cast<YieldOp>(block.back()).getOperation();
+  for (unsigned i = 0; i < yieldOp->getNumOperands(); ++i) {
----------------
This looks weird, why not just assert the back is a yield?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/LinalgToLoops.cpp:101
+  auto *yieldOp = cast<YieldOp>(block.back()).getOperation();
+  for (unsigned i = 0; i < yieldOp->getNumOperands(); ++i) {
+    std_store(map.lookup(yieldOp->getOperand(i)), outputBuffers[i],
----------------
nit: Please cache the end iterator of the loop. This is the LLVM style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76413





More information about the llvm-commits mailing list