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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
