[flang-commits] [PATCH] D144743: [flang] Fix lowering of optional char proc args

[Jean Perier via Phabricator via flang-commits]

jeanPerier added a comment.

Thanks a lot, I think the IR can be slightly further simplify by merging the fir.absent and fir.emboxproc into a single fir.absent. Looks great otherwise.



================
Comment at: flang/lib/Optimizer/Builder/FIRBuilder.cpp:619
+                                           mlir::Type argTy) {
+  if (!fir::isCharacterProcedureTuple(argTy)) {
+    return create<fir::AbsentOp>(loc, argTy);
----------------
Nit: single line if/for... should not have braces in LLVM/MLIR coding style (it's a bit of brain gymnastic...)

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


================
Comment at: flang/lib/Optimizer/Builder/FIRBuilder.cpp:623-627
+  auto boxTy =
+      argTy.cast<mlir::TupleType>().getType(0).cast<fir::BoxProcType>();
+  mlir::Type funcTy = boxTy.getEleTy();
+  mlir::Value funcAddr = create<fir::AbsentOp>(loc, funcTy);
+  auto boxProc = create<fir::EmboxProcOp>(loc, boxTy, funcAddr);
----------------
Thinking this through a bit more, doesn't `auto boxProc = create<fir::AbsentOp>(loc, argTy.cast<mlir::TupleType>().getType(0));` works an save the EmboxProc at that level?
I think AbsentOp should work properly on BoxProcType (otherwise, there would also be a bug for optional non character dummy procedures).

It will probably also make the extension of this code to optional character pointer procedure easier when they arrive (nothing to do other than updating the `isCharacterProcedureTuple` to detect them).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144743