[PATCH] D120403: Lower Fortran intrinsic to a runtime call/llvm intrinsic

[Eric Schweitz via Phabricator via llvm-commits]

schweitz added inline comments.


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:80
+  mlir::Value genAbs(mlir::Type, llvm::ArrayRef<mlir::Value>);
+  mlir::Value genIand(mlir::Type, llvm::ArrayRef<mlir::Value>);
   /// Define the different FIR generators that can be mapped to intrinsic to
----------------
I assume we want to add some doxygen commentary to these, albeit brief. (Noting that some of these member functions have it and some do not.)


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:282
+      return Conversion::None;
+    }
+    if (auto fromIntTy{from.dyn_cast<mlir::IntegerType>()}) {
----------------
Extra unneeded braces used throughout this function.


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:321
+
+  std::array<int, dataSize> conversions{/* zero init*/};
+  bool infinite{false}; // When forbidden conversion or wrong argument number
----------------
I think this will use the default initializer without explicitly stating it, no?


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:322
+  std::array<int, dataSize> conversions{/* zero init*/};
+  bool infinite{false}; // When forbidden conversion or wrong argument number
+};
----------------
nit: use `=`


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:346
+  auto range = lib.equal_range(name);
+  for (auto iter{range.first}; iter != range.second && iter; ++iter) {
+    const auto &impl = *iter;
----------------
nit: `=`


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:351
+      return getFuncOp(loc, builder, impl); // exact match
+    } else {
+      FunctionDistance distance(funcType, implType);
----------------
nit: LLVM style else after return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120403