[PATCH] D112445: [fir] Add fir.array_access op

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

> FIR array ops are meant to deal with Fortran's copy-in/copy-out semantics. They introduce a separation of concerns between lowering the parse trees and doing dependence analysis in the front end or resorting to brute-force introduction of ubiquitous array copies.

Is there a documentation on this design? Maybe a design discussion on the mailing-list you could point me to?

Let's see if I understand correctly, starting from:

   %s = fir.shape %n, %m : (index, index) -> !fir.shape<2>
  // load the entire array 'a'
  %v = fir.array_load %a(%s) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shape<2>) -> !fir.array<?x?xf32>
  // update the value of one of the array value's elements
  // %r_{ij} = %f  if (i,j) = (%i,%j),   %v_{ij} otherwise
  %r = fir.array_update %v, %f, %i, %j : (!fir.array<?x?xf32>, f32, index, index) -> !fir.array<?x?xf32>
  fir.array_merge_store %v, %r to %a : !fir.ref<!fir.array<?x?xf32>>

Then adding an array and a load:

   %s = fir.shape %n, %m : (index, index) -> !fir.shape<2>
  // load the entire array 'a'
  %v = fir.array_load %a(%s) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shape<2>) -> !fir.array<?x?xf32>
  // update the value of one of the array value's elements
  // %r_{ij} = %f  if (i,j) = (%i,%j),   %v_{ij} otherwise
  %r = fir.array_update %v, %f, %i, %j : (!fir.array<?x?xf32>, f32, index, index) -> !fir.array<?x?xf32>
  fir.array_merge_store %v, %r to %a : !fir.ref<!fir.array<?x?xf32>>
  %p = fir.array_access %v, %I, %j : (!fir.array<?x?xf32>, index) -> !fir.ref<f32>
  %l = fir.load %p : !fir.ref<f32>

Is the same as the following where the array_access is moved before the store:

   %s = fir.shape %n, %m : (index, index) -> !fir.shape<2>
  // load the entire array 'a'
  %v = fir.array_load %a(%s) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shape<2>) -> !fir.array<?x?xf32>
  %p = fir.array_access %v, %i, %j : (!fir.array<?x?xf32>, index) -> !fir.ref<f32>
  // update the value of one of the array value's elements
  // %r_{ij} = %f  if (i,j) = (%i,%j),   %v_{ij} otherwise
  %r = fir.array_update %v, %f, %i, %j : (!fir.array<?x?xf32>, f32, index, index) -> !fir.array<?x?xf32>
  fir.array_merge_store %v, %r to %a : !fir.ref<!fir.array<?x?xf32>>
  %l = fir.load %p : !fir.ref<f32>

Is the same as the following where the load is also moved before the store:

   %s = fir.shape %n, %m : (index, index) -> !fir.shape<2>
  // load the entire array 'a'
  %v = fir.array_load %a(%s) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shape<2>) -> !fir.array<?x?xf32>
  %p = fir.array_access %v, %i, %j : (!fir.array<?x?xf32>, index) -> !fir.ref<f32>
  %l = fir.load %p : !fir.ref<f32>
  // update the value of one of the array value's elements
  // %r_{ij} = %f  if (i,j) = (%i,%j),   %v_{ij} otherwise
  %r = fir.array_update %v, %f, %i, %j : (!fir.array<?x?xf32>, f32, index, index) -> !fir.array<?x?xf32>
  fir.array_merge_store %v, %r to %a : !fir.ref<!fir.array<?x?xf32>>

Something I am not clear on, is am I allow to store to this ref or is it "constant"?
For example is this legal IR:

  %v = fir.array_load %a(%s) : (!fir.ref<!fir.array<?x?xf32>>, !fir.shape<2>) -> !fir.array<?x?xf32>
  %p = fir.array_access %v, %i, %j : (!fir.array<?x?xf32>, index) -> !fir.ref<f32>
  fir.store %scalar to %p : !fir.ref<f32> // store to the 



> It is fully understood that other solutions exist. Some were considered at length. This one meets the project schedule.

Sure, but when you're proposing to make tradeoff like that, it is useful to make it explicit and document them carefully (I. Mentioning the alternatives, etc. is also useful. In general I invite you to conduct such discussion on the mailing-list (or Discourse I don't know what's preferred).

I'm also in general looking to anticipate not getting into situations like the discussion in https://reviews.llvm.org/D109579


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112445