[flang-commits] [flang] [flang][Lower] Implement lowering for new expression kind used in explicit-shape-bounds-spec (PR #215403)

[via flang-commits]

ivanrodriguez3753 wrote:

Some important points and details.

1. Previously, `CollectUsedSymbolValuesHelper` was commented `// unreachable`. This is demonstrably false. However, it didn't matter because dummy arguments aren't checked for read-before-write warnings, and in this context, ROBEs (RankOneBoundElements) can only be checked in declarations. So it's a latent bug waiting to be discovered in other contexts but impossible in this (explicit-shape-bounds-spec) context since dummy arguments are specifically not checked for this warning. However, in the future when this feature is implemented in an ALLOCATE statement, we WILL need the correct behavior. The correct behavior is simply recursing to the base expression. That being said, this is already an unrelated bug in several contexts. Observe the following test case:

```
program p
  implicit none
  integer :: aBound = 3      ! used ONLY as ALLOCATE bound
  integer :: doLim  = 3      ! used ONLY as DO upper bound
  integer :: ifThr  = 1      ! used ONLY in IF condition
  integer :: subIdx = 1      ! used ONLY as array subscript in an expression (control)
  integer, allocatable :: arr(:)
  integer :: acc, i
  allocate(arr(aBound))
  arr = 0
  acc = 0
  do i = 1, doLim
    acc = acc + i
  end do
  if (acc > ifThr) print *, 'big'
  print *, arr(subIdx), acc
end program
```
Compiling with `-pedantic` gives unused variable warnings for `aBound`, `doLim`, and `ifThr`, which is a false positive. These variables are used (read), but the Scalar wrappers don't have a typedExpression to check uses for. So... that should be a separate discussion. See my fork [branch](https://github.com/llvm/llvm-project/commit/c88346a7260e281f8b8adcf3271a0e5f3b41e326) for a draft fix that fixes the behavior but hasn't been thoroughly reviewed and might be too big a hammer for this fix. In any cases, once that bug is fixed, the corrected implementation for `CollectUsedSymbolValuesHelper` will actually matter.

2. Modfile dump was previously preserving the base symbol (of rank-1 and constant extent) and just emitting that array instead of synthesized scalar elements. This was true even for foldable cases because I thought preserving the symbol mattered. After comparing to the old syntax, in a foldable test case, it seems we lose symbol info when dumping to modfile, so I followed that precedent. The following test case includes foldable and non-foldable cases for both syntaxes:
```
module test_param
contains
  subroutine test_param_bounds(dims_arg)
    integer, intent(in) :: dims_arg(3)
    integer, parameter :: dims(3) = [2, 3, 4]
    real :: a(dims), b(dims(1), dims(2), dims(3))
    real :: a_arg(dims_arg), b_arg(dims_arg(1), dims_arg(2), dims_arg(3))
    a(1,1,1) = 1.0
    b(1,1,1) = 1.0
    a_arg(1,1,1) = 1.0
    b_arg(1,1,1) = 1.0
  end subroutine
end module
```
Modfile test has been updated with this expectation. Foldable ROBEs now emit a scalar constant as if you had used the old syntax. Unfoldable ROBEs emit the new representation. 

3. It is quite obvious that the `gen` method is inefficient in that it generates the base array pointer for every dimension, and they are byte-identical copies. I was hoping the optimizer would get rid of this inefficiency, and it partially does! Furthermore, since the semantic analyzer will reject any extent greater than the max supported array rank, this inefficiency is capped at N = 15. I was hoping all 6 dimensions in the line
`real :: a_arg(dims_arg), b_arg(dims_arg(1), dims_arg(2), dims_arg(3))`
could be collapsed to a single load of the base array pointer, but arrays in Fortran can be noncontiguous and it doesn't seem that less than 3 loads is possible in this case, and in general, n loads for n distinct dimensions. If this is optimizable to a single load, I think it would be an optimization at a lower level than HLFIR or FIR. HLFIR to FIR optimizes the 6 loads to 3 because of the shared extents for each dimension. 

https://github.com/llvm/llvm-project/pull/215403