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

Valentin Clement via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 25 06:12:07 PST 2021


clementval added inline comments.


================
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));
+  };
----------------
kiranchandramohan wrote:
> Nit: This is either my lack of familiarity or a bug.
Yeah it looks odd. Let me double check that in fir-dev. 


================
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) {
----------------
kiranchandramohan wrote:
> 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?
Of course we can check more stuff. I was just trying to find a trade off between no test and time spent on upstreaming this. 


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