Hi Julian:
On 07.12.22 20:13, Julian Brown wrote:
I know that this was the case before, but can you move the mark:1 etc.
after 'tlink'? In that case all bitfields are grouped together.
Thanks for doing so.
I wonder whether that also rejects the following – which seems to be
valid. The 'map' goes to 'target' and the 'firstprivate' to
'parallel', cf. OpenMP 5.2, "17.2 Clauses on Combined and Composite
Constructs", [340:3-4 & 12-14]. (BTW: While some fixes went into 5.1
regarding this section, a likewise wording is already in 5.0.)
(Testing showed: it give an ICE without the patch and an error with.)
...and this patch avoids the error for combined directives, and
reorders the gfc_symbol bitfields.
All in all, I am fine with the patch - but I spotted some issues.
First, I think you need to set for some error cases mark = 0 to avoid
duplicated errors.
Namely:
! Outputs the error twice ('Symbol ‘y’ present on multiple clauses')
!$omp target has_device_addr(y) firstprivate(y)
block; end block
* * *
Additionally, I think it would be good to have besides 'target' +
map/firstprivate (→ error)
also a testcase for 'target simd' + map/firstprivate → error
And I think also gives-no-error checks all combined 'target ...' that take
firstprivate
should be added, cf. your own patch - possibly with looking at the original
dump (scan-tree-dump)
to see that the clause is properly attached correctly. Example for 'target
teams':
!$omp target teams map(x) firstprivate(x)
block; end block
(Works but no testcase.)
* * *
The following is not diagnosed and gives an ICE:
!$omp target in_reduction(+: x) private(x)
block; end block
end
The C testcase properly has:
error: ‘x’ appears more than once in data-sharing clauses
Note: Using 'firstprivate' instead of 'private' shows the proper error also in
Fortran.
The following does not ICE but does not make sense (and is rejected in C):
4 | #pragma omp target private(x) map(x)
vs.
!$omp target map(x) private(x)
block; end block
(The latter produces "#pragma omp target private(x.0) map(tofrom:*x.0)", ups!)
* * *
I also note that 'simd' accepts private such that
#pragma omp target simd private(x) map(x)
for (int i=0; i < 0; i++)
;
!$omp target simd map(x) private(x)
do i = 1, 0; end do
is valid. (It is accepted by gcc and gfortran, i.e. it just needs to be added
as testcase.)
* * *
I note that C rejects {map(x),firstprivate(x)} +
{has_device_addr(x),is_device_ptr(x)}',
but gfortran + your patch accepts:
!$omp target map(x) has_device_addr(x)
!$omp target map(x) is_device_ptr(x)
while
!$omp target firstprivate(x) has_device_addr(x)
!$omp target firstprivate(x) is_device_ptr(x)
is rejected – showing the error message twice.
Expected: I think it should show an error in all four cases - but only once.
2022-12-06 Julian Brown <jul...@codesourcery.com>
gcc/fortran/
PR fortran/107214
* gfortran.h (gfc_symbol): Add data_mark, dev_mark, gen_mark and
reduc_mark bitfields.
* openmp.cc (resolve_omp_clauses): Use above bitfields to improve
duplicate clause detection.
gcc/testsuite/
PR fortran/107214
* gfortran.dg/gomp/pr107214.f90: New test.
Thanks,
Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
