Sandra Loosemore wrote:
For namelist entries in MAP etc clauses,
I think namelist is rather confusing.
As written in the patch comment to 5/10 (later written, but much earlier
sent) 'namelist' has a specific meaning in Fortran (I/O; fancy way to
write/read data in text form of a derived type).
And it does not have a meaning in OpenMP.
However, the name pops up in the Fortran FE as the
gfc_namelist and gfc_omp_namelist – which is a list of gfc_symbol
in the former case – and of gfc_symbol/gfc_expr and space to
store clause modifiers in the latter case.
I think it is best to talk about 'gfc_omp_namelist':
"For gfc_omp_namelist entries in MAP etc. clauses,"
That's a bit bulky but makes clear what is meant.
* * *
As mentioned for v7's 02/07, the included testcase
gcc/testsuite/gfortran.dg/gomp/target-map-iterators-5.f90
actually uses the code added in 02/07 - such that that patch
now actually has an associated testcase :-)
* * *
However, when I compile that testcase with -Wall, I see:
gcc/testsuite/gfortran.dg/gomp/target-map-iterators-5.f90:15:94:
warning: ‘omp_iter_data.19’ is used uninitialized [-Wuninitialized]
15 | !$omp target map(to: x) map(iterator(i=lbound(x, 1):ubound(x,
1):stride), to: x(i)%ptr(:))
-fdump-tree-gimple shows:
omp_iter_data.20_36 = omp_iter_data.19;
iftmp.21_37 = (void *) iftmp.8;
*omp_iter_data.20_36 = iftmp.21_37;
omp_iter_data.10_20 = omp_iter_data.9;
iftmp.11_21 = (void *) iftmp.8;
*omp_iter_data.10_20 = iftmp.11_21;
omp_iter_data.13_22 = omp_iter_data.12;
iftmp.14_23 = (void *) iftmp.8;
*omp_iter_data.13_22 = iftmp.14_23;
omp_iter_data.16_24 = omp_iter_data.15;
iftmp.17_25 = (void *) iftmp.8;
*omp_iter_data.16_24 = iftmp.17_25;
There is always an assignment of one 'omp_iter_data' to another,
where the RHS one is uninitialized, immediately followed by a
new assignment to the LHS using the initialized iftmp.8 variable.
This issue seems to be preexisting (i.e. not added in this patch),
but still it should be fixed – and relates to 'omp_iter_data' which
is on topic of this series.
* * *
+/* Scan through all clauses in LIST_P using OpenMP iterators. If any
+ clauses have iterators with variables that are not used (directly or
+ indirectly) by the clause decl or size, issue a warning and replace
+ the iterator with a copy with the unused variables removed. */
Also preexisting, but touched: You removed the warning due to false
positives. I think the current code would remove a lot of the false
positives - but in any case, there is no warning generated at all.
Thus: Can you remove the "issue a warning and"? Or tell me that I missed
something ;-)
* * *
New libgomp/testsuite/libgomp.fortran/pr127616.f90 test:
There is the following wording in the testcase, which no longer
seems to be true. (First, from reading the code and secondly, there
is no respective dg-* clause.)
+! Expected to FAIL until the remove_unused_omp_iterator_vars fix lands.
Can you fix the comment? (Including checking the wording in the
paragraph before.)
BTW: Thank/Kudos to Paul-Antoine for finding the issue and filing
a PR with a useful testcase and description!
* * *
namelist entry as well
as direct references to iterator variables.
(Another occurrence of "namelist" that needs to be updated.
I still a have no idea what it refers to.)
Otherwise: LGTM.
Thanks for the patch and sorry for the very belated review!
Tobias
gcc/ChangeLog
PR fortran/127616
* gimplify.cc (struct tempvar_deps): New.
(struct iterator_set_deps): New.
(count_modify_exprs): New.
(init_iterator_set_deps): New.
(free_iterator_set_deps): New.
(deps_contains_ivar): New.
(deps_contains_tvar): New.
(add_var_to_ivars): New.
(add_var_to_tvars): New.
(collect_iterator_dependencies): New.
(get_iterator_set_deps): New.
(mark_tvar_used): New.
(tvar_is_used): New.
(filter_side_effect_statements): New.
(remove_unused_omp_iterator_vars): Track indirect dependencies
on iterator variables through side-effect statements. Do not try
to share iterator loops between clauses that may have different
statement lists. Remap iterator variable references in the
side-effect statements the same as for the decl and size
expressions.
gcc/testsuite/ChangeLog
PR fortran/127616
* gfortran.dg/gomp/target-map-iterators-5.f90: New.
libgomp/ChangeLog
PR fortran/127616
* testsuite/libgomp.fortran/pr127616.f90: New.
