[PATCH] D115797: [Flang][NFC] Add test with shape for allocmem and freemem

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski added a comment.

Thank you @kiranchandramohan ! A few micro nits,  otherwise LGTM!



================
Comment at: flang/test/Fir/convert-to-llvm.fir:206
+
+func @test_with_shape(%arg0: !fir.ref<i32>, %arg1: !fir.ref<i32>) {
+  %0 = fir.load %arg0 : !fir.ref<i32>
----------------
Could `%arg0` and `%arg1` be `index` instead? Wouldn't that make this test much simpler?


================
Comment at: flang/test/Fir/convert-to-llvm.fir:218
+// CHECK-SAME: %[[ARG0:.*]]: !llvm.ptr<i32>, %[[ARG1:.*]]: !llvm.ptr<i32>
+// CHECK:   %[[M:.*]] = llvm.load %[[ARG0]] : !llvm.ptr<i32>
+// CHECK:   %[[M_EXT:.*]] = llvm.sext %[[M]] : i32 to i64
----------------
[nit] `M` is a bit enigmatic without the original source code. You could rename as `N_ROWS` (and `N` as `N_COLS`).


================
Comment at: flang/test/Fir/convert-to-llvm.fir:250
+// CHECK:   %[[TOTAL_SIZE:.*]] = llvm.mul %[[LEN_SIZE]], %[[N_EXT]]  : i64
+// CHECK:   %[[MEM:.*]] = llvm.call @malloc(%6) {{.*}}bindc_name = "s"
+// CHECK:   %[[B1:.*]] = llvm.bitcast %[[MEM]] : !llvm.ptr<i8> to !llvm.ptr<i8>
----------------
It might be `LEN_SIZE` rather than `TOTAL_SIZE`, not sure!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115797