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

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added inline comments.


================
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
----------------
rovka wrote:
> Should we use backticks or something around function names? It reads really strange otherwise.
> Also, isn't this actually AllDim?
> Should we use backticks or something around function names? 
+1

Also, what is the difference between `genAllDescriptor` and `genAll`? Why do we need both?


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:14
+TEST_F(RuntimeCallTest, genAllTest) {
+  auto loc = firBuilder->getUnknownLoc();
+  mlir::Type seqTy =
----------------



================
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);
+}
----------------
Same suggestion in other places.


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/ReductionTest.cpp:25-26
+  auto loc = firBuilder->getUnknownLoc();
+  mlir::Type seqTy =
+      fir::SequenceType::get(fir::SequenceType::Shape(1, 10), i32Ty);
+  mlir::Value result = firBuilder->create<fir::UndefOp>(loc, seqTy);
----------------
Move next to other "// Commonly used types" in `RuntimeCallTest`?


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h:21
+    mlir::OpBuilder builder(&context);
+    auto loc = builder.getUnknownLoc();
+
----------------



================
Comment at: flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h:87-93
+  for (auto &u : result.getUses()) {
+    if (mlir::isa<fir::CallOp>(*u.getOwner())) {
+      checkCallOp(u.getOwner(), fctName, nbArgs);
+    } else {
+      auto convOp = mlir::dyn_cast<fir::ConvertOp>(*u.getOwner());
+      checkCallOpFromResultBox(convOp.getResult(), fctName, nbArgs);
+    }
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114460