[PATCH] D74172: [mlir][Linalg] Implement fusion of linalg.generic operation on tensors.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:426
+  auto fusedLinalgOp = b.create<GenericOp>(
+      UnknownLoc::get(context), consumerOp.getResultTypes(), fusedOperandsList,
+      b.getI64IntegerAttr(fusedArgsIn), b.getI64IntegerAttr(fusedArgsOut),
----------------
Is there any other location you can use besides unknown? We should try to propagate location as much as possible.


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:437
+  Block &consumerOpBlock = consumerOp.region().front();
+  Block *fusedBlock = new Block();
+  fusedOpRegion.push_back(fusedBlock);
----------------
fusedBlock = b.createBlock(&fusedOpRegion); 

?


================
Comment at: mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp:522
+    // Find the first operand that is defined by another generic op on tensors.
+    for (auto operand : llvm::enumerate(op.getOperation()->getOperands())) {
+      auto definingOp =
----------------
Are all of these .getOperation calls really necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74172