[PATCH] D113551: [fir] Add fir.box_rank, fir.box_addr, fir.box_dims and fir.box_elesize conversion

Wed Nov 10 02:24:19 PST 2021

clementval added inline comments.

================
Comment at: flang/lib/Optimizer/CodeGen/CodeGen.cpp:32-35
+static constexpr unsigned kAddrPosInBox = 0;
+static constexpr unsigned kEleSizePosInBox = 1;
+static constexpr unsigned kRankPosInBox = 3;
+static constexpr unsigned kDimsPosInBox = 7;
----------------
clementval wrote:
> awarzynski wrote:
> > It's great to see some non-magic values used here! Do these numbers correspond to https://github.com/llvm/llvm-project/blob/99ad2079d452f587be050b3867e0ed4856335fb2/flang/lib/Optimizer/CodeGen/TypeConverter.h#L105-L116? If yes, could these be used for the type converter too? Also, could you add a comment here that would explain the origins of these indices? Ta!
> Yes they are the same. Let me do a quick patch to move those value in TypeConverter and use them. I'll base this patch on it. 
Done in D113553

================
Comment at: flang/test/Fir/convert-to-llvm.fir:646
+// CHECK-LABEL: llvm.func @extract_rank(
+// CHECK-SAME:                          %[[ARG0:.*]]: !llvm.ptr<struct<(ptr<f64>, i64, i32, i8, i8, i8, i8)>>) -> i32
+// CHECK:         %[[C0:.*]] = llvm.mlir.constant(0 : i32) : i32
----------------
awarzynski wrote:
> Shouldn't this be written in a target agnostic way? Like here: https://github.com/llvm/llvm-project/blob/99ad2079d452f587be050b3867e0ed4856335fb2/flang/test/Fir/types-to-llvm.fir#L38 Same comment for other tests here.
Yeah might be safer. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113551