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

[Diana Picus via Phabricator via llvm-commits]

rovka added a comment.

Just some nits, otherwise LGTM (but please wait for Andrzej and Kiran to have another look too).



================
Comment at: flang/include/flang/Optimizer/Builder/Runtime/Reduction.h:21
+
+/// Generate call to all runtime routine.
+/// This calls the descriptor based runtime call implementation of the `all`
----------------



================
Comment at: flang/include/flang/Optimizer/Builder/Runtime/Reduction.h:28
+
+/// Generate call to any runtime routine.
+/// This calls the descriptor based runtime call implementation of the `any`
----------------



================
Comment at: flang/lib/Optimizer/Builder/Runtime/Reduction.cpp:454
+
+/// Generate call to all runtime routine.
+/// This calls the descriptor based runtime call implementation of the `all`
----------------



================
Comment at: flang/lib/Optimizer/Builder/Runtime/Reduction.cpp:464
+
+/// Generate call to any runtime routine.
+/// This calls the descriptor based runtime call implementation of the `any`
----------------



================
Comment at: flang/lib/Optimizer/Builder/Runtime/Reduction.cpp:553
+  else if (eleTy.isF80())
+    func = fir::runtime::getRuntimeFunc<ForcedMaxvalReal10>(loc, builder);
+  else if (eleTy.isF128())
----------------
What happens if one of these is actually used before it's implemented in the runtime? Will that be a linker error? Or is it caught sooner?


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:224
+
+void checkGenMxxloc(fir::FirOpBuilder &builder,
+    void (*genFct)(fir::FirOpBuilder &, Location, mlir::Value, mlir::Value,
----------------



================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:250
+
+void checkGenMxxlocDim(fir::FirOpBuilder &builder,
+    void (*genFct)(fir::FirOpBuilder &, Location, mlir::Value, mlir::Value,
----------------



================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:279
+
+void checkGenMxxvalChar(fir::FirOpBuilder &builder,
+    void (*genFct)(
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114460