[PATCH] D114460: [fir] Add fir reduction builder

[Valentin Clement via Phabricator via llvm-commits]

clementval added inline comments.


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:20
+  mlir::Value all = fir::runtime::genAll(*firBuilder, loc, undef, dim);
+  checkCallOp(all.getDefiningOp(), "_FortranAAll", 2);
+}
----------------
awarzynski wrote:
> clementval wrote:
> > awarzynski wrote:
> > > clementval wrote:
> > > > awarzynski wrote:
> > > > > Same suggestion in other places.
> > > > Does this really help?
> > > Yes. `_FortranAll` is somewhat self-explanatory. But `2` is a magic number without the comment.
> > Fells like more work for not so much
> Not that much work:
> 
> **Add `/*fctName=*/ **
> ```lang=bash
> awk '($1 ~ /checkCallOpFromResultBox/) && !($2 ~ /fctNam/) {printf("%s /*fctName=*/%s %s\n", $1, $2, $3)}' flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp
> ```
> 
> **Add `/*nbArgs=*/ **
> ```lang=bash
> awk '($1 ~ /checkCallOpFromResultBox/) && !($3 ~ /nbArgs/) {printf("%s %s /*nbArgs=*/%s\n", $1, $2, $3)}' flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp
> ```
> 
> This is not a blocker for me.
Editing is quite easy as you mentioned. But updating all the patches is time consuming. I personally don't see the value of adding this. If someone else lean on your side I'll do the update. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114460