Sandra Loosemore wrote:
Tobias originally reported and tracked down this ICE when reviewing
one of Kwok's iterators patches that isn't on mainline yet:
https://gcc.gnu.org/pipermail/gcc-patches/2025-September/695447.html
The bug isn't in that patch, though; it's in an earlier patch in the
series that has already been approved and committed.
The attached patch fixes it, and adds a standalone test case. OK for
GCC 16?
The change LGTM. Thanks for taking care of this!
I wonder whether we should add a runtime test for this as well.
Currently, GCC has:
(A) libgomp/testsuite/libgomp.c-c++-common/target-map-iterators-1.c
this tests 'target'.
(B) libgomp/testsuite/libgomp.c-c++-common/target-map-iterators-2.c
(C) libgomp/testsuite/libgomp.c-c++-common/target-map-iterators-3.c
Those test 'target enter data' and 'target update' with 'from' (B)
and 'to' (C).
As those tests show, only 'target data' ICEs, now fixed by this patch;
'target' and 'target enter/exit data' work.
[Actually, I miss a check for 'target exit data' as -2 and -3 only
use 'target enter data'; either using the 'release' or the 'from'
map modifier. It seems to make sense to explicitly check 'exit data'
as the code path is not completely the same as for 'enter data'.]
Regarding the added testcase,
gcc/testsuite/c-c++-common/gomp/target-map-iterators-6.c:
In terms of a GCC test case, makes sense: no ICE and dump is fine.
However, as user code it does not make sense; first, it is a bit
unclear whether everything required is already mapped by the caller
(granted, we don't see the caller) - and inside 'target data' it
modifies 'x' on the host.
I wonder whether we want to leave it as is or to make it a bit more
sensible, e.g., by adding an 'omp target' before the loop or by
replacing the loop by a function call (leaving it to the exercise of
the reader to guess what that function does with 'x').
* * *
Thanks,
Tobias