[flang-commits] [PATCH] D139351: Lowering and runtime support for F08 transformational intrinsics: BESSEL_JN and BESSEL_YN

[Slava Zakharin via Phabricator via flang-commits]

vzakhari added inline comments.


================
Comment at: flang/lib/Lower/IntrinsicCall.cpp:2690
+    mlir::Value n2_1 = builder.create<mlir::arith::SubIOp>(loc, n2, one);
+    mlir::Value bn2_1 = builder.create<mlir::arith::SelectOp>(
+        loc, n1LtN2, genRuntimeCall("bessel_jn", resultType, {n2_1, x}), zero);
----------------
I think unconditional computation of the recursion bases might be suboptimal at least for the `n1 == n2` case.  Can you please rewrite the selects with `IfOp`?


================
Comment at: flang/lib/Optimizer/Builder/Runtime/Transformational.cpp:31
+      auto strTy = fir::ReferenceType::get(mlir::IntegerType::get(ctx, 8));
+      auto intTy = mlir::IntegerType::get(ctx, 8 * sizeof(int));
+      auto noneTy = mlir::NoneType::get(ctx);
----------------
For portability, I think it is better to use `32` here and `int32_t` in the runtime implementation.


================
Comment at: flang/runtime/transformational.cpp:168
+
+  if (n2 < n1) {
+    return;
----------------
Should we also verify that `n1` and `n2` are non-negative?


================
Comment at: flang/runtime/transformational.cpp:193
+
+  if (n2 > n1) {
+    at[0] = n2 - n1;
----------------
Early return is more preferable.


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

https://reviews.llvm.org/D139351