[PATCH] D114159: [flang][codegen] Add a conversion for `fir.coordinate_of` - part 1

Andrzej Warzynski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 9 02:06:47 PST 2021


awarzynski added a comment.

In D114159#3182242 <https://reviews.llvm.org/D114159#3182242>, @rovka wrote:

> Seems fine, thanks.
>
> In D114159#3175939 <https://reviews.llvm.org/D114159#3175939>, @awarzynski wrote:
>
>> Allow dynamically sized arrays inside `!fir.box`.
>>
>> I mistakenly disallowed dynamically sized arrays earlier. However, IIUC, boxed arrays (and that's the only type of arrays dealt with in this patch) shouldn't require size when using `fir.coordinate_of`. In fact, `!fir.box<!fir.array<? x i32>>` should be no different to `!fir.box<!fir.array<? x i32>>` in this case.
>
> I assume you mean !fir.ref<!fir.array<? x i32>> versus !fir.box<!fir.array<? x i32>> :)

I meant that `!fir.box<!fir.array<10 x i32>>` and `!fir.box<!fir.array<? x i32>>` should be identical :) That was the source of confusion for me initially and it finally clicked when working on https://reviews.llvm.org/D115333. Basically, I thought that ` !fir.box<!fir.array<? x i32>>` shouldn't work and that's why I added the extra check in `doRewrite`. However, since `!fir.array` is "boxed" (that's the only case considered here), this should be just fine and so I removed that check.



================
Comment at: flang/test/Fir/convert-to-llvm.fir:2078
+func @coordinate_of_box_dynamic_array_1d(%arg0: !fir.box<!fir.array<? x f32>>, %arg1: index) {
+// expected-error at +1{{failed to legalize operation 'fir.coordinate_of'}}
+  %p = fir.coordinate_of %arg0, %arg1 : (!fir.box<!fir.array<? x f32>>, index) -> !fir.ref<f32>
----------------
rovka wrote:
> Do we still expect an error here?
This particular line was copied by mistake, sorry! It will be ignored as we don't use `-verify-diagnostics` in this file.

Instead, the code is correctly converted (see the expected output below). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114159



More information about the llvm-commits mailing list