[PATCH] D114557: [fir] Add base for runtime builder unittests

Kiran Chandramohan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 25 06:00:11 PST 2021


kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
This revision is now accepted and ready to land.

Can `flang/lib/Lower/RTBuilder.h` be deleted now?

LGTM. Please wait for a day if there are comments outstanding in this patch or before it was refactored.



================
Comment at: flang/include/flang/Optimizer/Builder/Runtime/RTBuilder.h:141
+  return [](mlir::MLIRContext *context) -> mlir::Type {
+    return mlir::IntegerType::get(context, 8 * sizeof(std::size_t));
+  };
----------------
Nit: This is either my lack of familiarity or a bug.


================
Comment at: flang/unittests/Optimizer/Builder/Runtime/RuntimeCallTestBase.h:77
+/// added in this check when \p addLocArgs is true.
+static void checkCallOp(mlir::Operation *op, llvm::StringRef fctName,
+    unsigned nbArgs, bool addLocArgs = true) {
----------------
Nit: would it make sense to check the return value or return type since some runtime functions return an mlir::value and others return in the descriptor passed as an argument?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114557



More information about the llvm-commits mailing list