Yuao Ma wrote:
I've got a quick question about the collapse clause in OpenMP. I've been looking at the standard, specifically section 6.4.5, and it says that the value of 'n' can't be greater than the loop nest depth. But it doesn't seem to specify what kind of expression you can use for 'n' in C++ or Fortran.
It must be a constant positive integer; see OpenMP 5.1. In OpenMP 5.2, clause description moved from the associated directive into a section per clause – and, internally, code was moved from the TeX file to a JSON file and generator. Some additional code moved to JSON + generator in 6.0. — Like with all consolidation and restructuring, bugs are found and fixed on the way — but new issues are created.
In this particular case, the issue was fixed on July 2, 2025 in the internal* git repository – such that OpenMP TR13 (nex TR, TR = Technical Report), due November 2025, will have the fix. There is also the plan to create an errata document – and this item is a hot candidate.
(*Unfortunately, the git repository and specification issue tracker of the OpenMP specification are only internally accessible by OpenMP members. On the other hand, at least, there is a draft document (TR) every year – and the final specification is also publicly available.)
* * *
From a compiler writer's perspective, how should we handle this? Maybe use any valid integer expression that resolves correctly? Or are there other rules I might've missed?
The loop check is done in openmp.cc's 'resolve_omp_do' ("not enough DO loops for collapsed") - and the code assumes that the spec requires constant positive integers (as it did and now does again); I have not checked whether resolve_positive_int_expr is called for 'resolve' or not - because that checked for non-negative (it still requires a constant check – possibly afterwards as resolution happens inside that function; moving it to resolution time instead of parser time is more Fortran like).
* * *
Thank you for the thorough review! I think the patch is okay for trunk now(Bootstrapped and regression tested on an x86_64 Linux)?
Yes, that's what I wanted to convey by 'LGTM' (looks good to me) which is besides 'OK' one of the two ways to imply that the patch is approved* for committing it to mainline.
(*Unless qualified, e.g., by someone pointing out that he cannot approve it [→ not a maintainer/reviewer for that code part, cf. MAINTAINERS file] or when approving additionally for some branches [regression backporting].)
I will start working on the nil part.
Thanks! I missed this feature in the past and I am happy that it is now supported by the Fortran language and by gfortran – at least for the most common usage, handling all other cases will take a while but with .nil., yet another case will be handled :-)
Tobias
