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

Alex Zinenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 15 04:29:29 PST 2020


ftynse added a comment.

@mehdi_amini @nicolasvasilache @herhut  Let's take some time and discuss builder APIs outside this diff (also involving @rriddle). My basic observations are that (1) writing structured IR, as in "with nested regions", looks unnecessarily complicated with builders, arguments are the same as those against goto-style programming; (2) a lot of IR construction internally happens in rewrite patterns, where location almost always remains the same, that of the matched operation root; (3) current EDSC APIs are contentious partly because it is unclear when reading the code when the function call creates the IR vs. when it's just a function call.

@csigg I'm fine accepting this, but the test is currently broken. Please fix and we'll be ready to land.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72129





More information about the llvm-commits mailing list