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

Kiran Chandramohan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 16 05:38:23 PST 2021


kiranchandramohan marked 3 inline comments as done.
kiranchandramohan added a comment.

Thanks @awarzynski. I have made the suggested changes.



================
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>
----------------
awarzynski wrote:
> Could `%arg0` and `%arg1` be `index` instead? Wouldn't that make this test much simpler?
Yes, it will make it a simpler test. I have made the change.

The original intent was to add a test that would model the input coming from the lowering and add the fortran source corresponding to it.


================
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
----------------
awarzynski wrote:
> [nit] `M` is a bit enigmatic without the original source code. You could rename as `N_ROWS` (and `N` as `N_COLS`).
Have changed to NROWS, NCOLS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115797



More information about the llvm-commits mailing list