[PATCH] D114472: [fir] Add fir derived type runtime builder

Wed Dec 1 19:24:48 PST 2021

rovka added inline comments.

================
Comment at: flang/lib/Optimizer/Builder/Runtime/Derived.cpp:25
+  auto args = fir::runtime::createArguments(builder, loc, fTy, box, sourceFile,
+                                            sourceLine);
+  builder.create<fir::CallOp>(loc, func, args);
----------------
This doesn't seem to match the [[ https://github.com/llvm/llvm-project/blob/4479138de8e662f0dc64a92008b126f050e18b77/flang/runtime/derived.h#L24 | prototype from the runtime]]

================
Comment at: flang/lib/Optimizer/Builder/Runtime/Derived.cpp:33
+  auto fTy = func.getType();
+  auto args = fir::runtime::createArguments(builder, loc, fTy, box);
+  builder.create<fir::CallOp>(loc, func, args);
----------------
This doesn't match the runtime prototype for Destroy (it needs a bool too).
Also, we probably need one of these for Finalize as well?

================
Comment at: flang/lib/Optimizer/Builder/Runtime/Derived.cpp:41
+                                        mlir::Value sourceBox) {
+  auto func = fir::runtime::getRuntimeFunc<mkRTKey(Assign)>(loc, builder);
+  auto fTy = func.getType();
----------------
Is this really needed? We already have one in https://reviews.llvm.org/D114475. Maybe Assign was moved from derived.h to assign.h and this wasn't updated?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114472