[flang-commits] [PATCH] D112910: [flang] Add TargetRewrite pass

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added a comment.

Thanks for sending this @rovka ! Please see my questions/suggestions inline.



================
Comment at: flang/lib/Optimizer/CodeGen/Target.h:74
+  /// A function that returns a `boxchar<n>` type value must already have
+  /// converted that return value to an sret argument. This requirement is in
+  /// keeping with Fortran semantics, which require the caller to allocate the
----------------
What is `sret`?


================
Comment at: flang/lib/Optimizer/CodeGen/TargetRewrite.cpp:56
+
+/// Target-specific rewriting of the IR. This is a prerequisite pass to code
+/// generation that traverses the IR and modifies types and operations to a
----------------
[nit] IR --> MLIR (I think that IR is too ambiguous here)


================
Comment at: flang/test/Fir/target.fir:5
+// RUN: tco --target=powerpc64le-unknown-linux-gnu --target-rewrite %s | FileCheck %s --check-prefix=INT64
+
+// INT32-LABEL: @boxcharparams
----------------
Could you add comments to highlight which cases from the pass implementation are tested? Based on comments in  TargetRewrite.cpp, I'd expect to see
* type conversion on signatures and call sites.
* ops conversion in target-specific patterns.


================
Comment at: flang/test/Fir/target.fir:53
+
+func private @callee(%x : !fir.boxchar<1>)
+
----------------
This signature is also modified - could you add a `CHECK` line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112910