[flang-commits] [flang] [llvm] [mlir] [flang][mlir][OpenMP] Allow integer host_eval uses in effect-free ops (PR #223475)

[Sergio Afonso via flang-commits]

https://github.com/skatrak requested changes to this pull request.

Thank you for this work. I have taken a look and have some fundamental concerns about the approach, though.

When a variable that we use for e.g. host-evaluated loop bounds of an SPMD kernel is also used in other ways, like being passed to functions, appear in expressions, etc. then that variable must also be mapped and its mapped value be the one that is used for those other purposes. This is what already happens if you use one of the loop bounds variables inside of an SPMD kernel:

```f90
!$omp target teams distribute
do i = A, B
  ! A and B here won't refer to the host_eval entry block args.
  call foo(A, B)
end do
```

`host_eval` is exclusively for values that must be calculated in advance on the host and used when launching the kernel itself. From what I can tell by looking at this patch, these new allowed cases for `host_eval` don't actually change how the trip count is calculated, so it doesn't seem like we need those uses to be allowed through that restricted path.

So, unless I'm missing something, I think this is not the right solution to the problem. The `bounds` SSA values used within the imperfectly nested collapsed loops to identify when to exit early should not be the `host_eval` ones, but other `map` or `firstprivate` for the same outside values. Otherwise, we're opening the door, as you have already stated yourself, to allowing some unintended cases through and relying on the frontend to not misuse the dialect. And I think there's no benefit we get out of it in exchange.

Do let me know if I'm missing something, though. Probably a complete MLIR representation of what is generated for a collapsed imperfectly nested loop on an SPMD target region would help me understand more quickly where I'm wrong, in that case.

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