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

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski 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;
----------------
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!


================
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
----------------
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.


================
Comment at: flang/test/Fir/convert-to-llvm.fir:653-656
+func @foo1(%arg0: !fir.box<f64>) -> i32 {
+  %0 = fir.box_rank %arg0 : (!fir.box<f64>) -> i32
+  return %0 : i32
+}
----------------
DELETEME?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113551