[PATCH] D115077: [fir] Add array operations documentation

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: flang/docs/FIRArrayOperations.md:94
+  fir.array_merge_store %v, %r to %a : !fir.ref<!fir.array<?x?xf32>>
+```
+
----------------
clementval wrote:
> mehdi_amini wrote:
> > It's not clear to me in this example why array_merge_store needs %v? 
> > Why isn't it just `fir.array_store %r to %a` enough? Seems like the array_update should have all the information here.
> In this example it has everything but it's not always the case like in the example below: 
> 
> ```
> func @conversion_with_temporary(%arr0 : !fir.ref<!fir.array<10xi32>>) {
>    %c10 = arith.constant 10 : index
>    %1 = fir.shape %c10 : (index) -> !fir.shape<1>
>    %2 = fir.array_load %arr0(%1) : (!fir.ref<!fir.array<10xi32>>, !fir.shape<1>) -> !fir.array<10xi32>
>    %c10_i64 = arith.constant 10 : i64
>    %3 = fir.convert %c10_i64 : (i64) -> index
>    %c1_i64 = arith.constant 1 : i64
>    %c-1_i64 = arith.constant -1 : i64
>    %4 = fir.shape %c10 : (index) -> !fir.shape<1>
>    %5 = fir.slice %c10_i64, %c1_i64, %c-1_i64 : (i64, i64, i64) -> !fir.slice<1>
>    %6 = fir.array_load %arr0(%4) [%5] : (!fir.ref<!fir.array<10xi32>>, !fir.shape<1>, !fir.slice<1>) -> !fir.array<10xi32>
>    %c1 = arith.constant 1 : index
>    %c0 = arith.constant 0 : index
>    %7 = arith.subi %3, %c1 : index
>    %8 = fir.do_loop %arg0 = %c0 to %7 step %c1 unordered iter_args(%arg1 = %2) -> (!fir.array<10xi32>) {
>      %9 = fir.array_fetch %6, %arg0 : (!fir.array<10xi32>, index) -> i32
>      %10 = fir.array_update %arg1, %9, %arg0 : (!fir.array<10xi32>, i32, index) -> !fir.array<10xi32>
>      fir.result %10 : !fir.array<10xi32>
>    }
>    fir.array_merge_store %2, %8 to %arr0 : !fir.array<10xi32>, !fir.array<10xi32>, !fir.ref<!fir.array<10xi32>>
>    return
>  }
> ```
I don't quite get the different in this example?
If I track the loop, you have the loaded array used a initial induction value, then you update one element, and then pass it along to the next iteration until the end. %8 seems to contain the array fully updated?


================
Comment at: flang/docs/FIRArrayOperations.md:122
+
+It is only possible to use `array_fetch` on an `array_load` result value.
+
----------------
clementval wrote:
> mehdi_amini wrote:
> > Why this restriction?
> The array "abstraction" in FIR is not meant to be used anywhere. Fortran itself doesn't have "array values" in any sane sense of that term. To avoid to introduce arrays universally in FIR we restrict the use of the abstraction to precisely where we want it and where it is useful. 
> 
I don't feel this is a clear answer. What does "To avoid to introduce arrays universally" means for example?
Restricting the use of array makes sense: that means that `fir.add` will not accept to add two arrays for example, but that does not explain clearly to me the link between fetch and load.


In general: an SSA value is equal to another SSA value, what you're saying here is that inside the IR this isn't the case and a transformation that would make an SSA value a block argument for example isn't valid.

This seems fragile and extremely uncommon (I don't know of any precent in any dialect I worked on), so I'd insist to look for any alternative to such a strong design constraint: the bar to justify this should be really high and we're not there yet IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115077