[PATCH] D72129: [mlir] Add in-dialect lowering of gpu.all_reduce.

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/GPU/Transforms/AllReduceLowering.cpp:162
+      return getFactory(reduceOp.body());
+    if (reduceOp.op())
+      return getFactory(*reduceOp.op());
----------------
`auto op = reduceOp.op()` to avoid recomputing below.


================
Comment at: mlir/lib/Dialect/GPU/Transforms/AllReduceLowering.cpp:300
+                [&] {
+                  return llvm::SmallVector<Value , 1>{
+                      accumFactory(value, shuffleOp.getResult(0))};
----------------
Remove `llvm::` from most of the things in this file. They are re-exported in the mlir namespace already.


================
Comment at: mlir/lib/Dialect/GPU/Transforms/AllReduceLowering.cpp:352
+    };
+    while (funcOp.walk(callback).wasInterrupted()) {
+    }
----------------
When does this ever fail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72129