[clang] [llvm] [OpenMP] Diagnostic check for imperfect loop collapse (PR #96087)

[Michael Kruse via cfe-commits]

Meinersbur wrote:

I agree with @kparzysz that the examples you are using are generally unsafe. E.g. with
```c
  #pragma omp parallel for collapse(2)
    for (int i = 0; i < N; i++) {
      arr[i][i] = ...;
      for (int j = 0; j < N; j++) {
        arr[i][j] = ...;
      }
    }
```
there shouldn't be an expectation on which thread `arr[i][i]` is executed relative to all `a[i][j]`. The use case the non-perfectly nested loop are intended for is to to a precomputation that only depends on i, not on j:
```c
  #pragma omp parallel for collapse(2)
    for (int i = 0; i < N; i++) {
      double e = exp(2*PI*i);
      for (int j = 0; j < N; j++) {
        arr[i][j] = ... e ...;
      }
    }
```
A compiler could optimize this by computing `exp(2*PI*i)` at most once per thread that is scheduled at least one chunk that uses `i`, but computing it for any inner loop iteration is just as correct.

I agree with @alexey-bataev that this should be something handled as a diagnostic in the frontend, like [`Sema::checkOpenMPLoop`](https://github.com/llvm/llvm-project/blob/d6d8243dcd4ea768549904036ed31b8e59e14c73/clang/lib/Sema/SemaOpenMP.cpp#L9620). Pass analysis messages are intended for optimization hints, and are unreliable for correctness warnings (they change depending on optimization levels and previous optimizations, e.g. some code may have been optimized away because of undefined behavior (like ... race conditions), but still would want to warn about it). Point-to analysis would be out-of-scope at this level. I would recommend to only emit the warning if some non-private memory is being written to.  If being more sophisticated maybe instead whether `i` is not used to access the global or the same pointer/array variable used used somewhere else in the body. That includes the in-between code as well since it is sunk into the inner loop. 

Note that the Clang community is very conservative about adding new warnings. Generally, they don't want to add new warnings that are not enabled by default, must not have a legitimate use-case ([adding a warning for calling a virtual method in a constructor was turned down because of this](https://discourse.llvm.org/t/warning-when-calling-virtual-functions-from-constructor-desctructor/50589)), and false-positives must have a form that tells the compiler that this is intended such as `if ((x = 5))`. Everything beyond this should go into clang-tidy or the static analyzer.

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