Hi PA,
thanks for the updated patch.
I added fortran@ as it touches also Fortran, albeit only for
OpenMP-specific code.
Paul-Antoine Arras wrote:
In summary:
- Make clear in the changelog that this is about non-executable
directives not (only) about 'nothing' (even if only a subset
is handled).
Reworded accordingly.
Namely,
As permitted by OpenMP 6, accept a subset of non-executable directives -- namely
'metadirective', 'nothing', 'assume' and 'error at(compilation)' -- in
intervening code.
I think it would be helpful to make the distinction between
the spec and what is implemented, e.g.
"OpenMP 6 permits non-executable directives in intervening code;
this commit adds support for a sensible subset, namely
metadirectives, nothing, assume, and 'error at(compilation)'."
* * *
Actually, when writing this I used on purpose the wording
'metadirectives' - because there is also
begin metadirective ... end metadirective
Thus, I ended up trying it.
For C/C++, I think it is not implemented - it only makes sense with
those directives that take an end directive and those are all of
declarative nature - hence, we excluded it, even if one can construct
mildly sensible cases.
However, for Fortran it is - unsurprisingly, as it has tons of
end directives.
* * *
I was suggesting that you try it + add a testcase, but I am running
into the issue that for '.false.' the 'end metadirective' is no
longer parsed → https://gcc.gnu.org/PR122306 (comment 2)
However, this seems to be unrelated to your patch. Hence,
no action is required on your side.
* * *
- Can you check why gfc_match_omp_context_selector's call to
if (!gfc_resolve_expr (otp->expr)
|| (property_kind == OMP_TRAIT_PROPERTY_BOOL_EXPR
&& otp->expr->ts.type != BT_LOGICAL)
failed to simply the conditional expression?
After running into GDB, my understanding is that gfc_resolve_expr
detects that the type is BT_LOGICAL but does not try to compute the
value:
(gdb) p otp->expr->ts.type
$18 = BT_LOGICAL
(gdb) p otp->expr->value.logical
$13 = 72631568
looks like a random uninitialised value.
Hmm, actually, when I tried it was OK, but as 'gfc_debug_expr'
showed here: '(.false. ? x_min < 0 : .false.)'
which can be evaluated as constant - but isn't.
Cf. also https://gcc.gnu.org/pipermail/gcc-patches/2025-October/698170.html
where I wrote this to Yuao, who is taking care of that
conditional-expression feature.
In any case, it is unrelated to your patch.
* * *
+ /* If only one selector matches and it evaluates to 'omp nothing', no need to
+ * proceed. */
GCC does not seem to use this comment style in any file.
Namely, there is no '* ' at the beginning of consecutive comment lines.
Can you fix it?
* * *
+++ b/gcc/c/c-parser.cc
[…]
+ && id >= PRAGMA_OMP__START_ && id <= PRAGMA_OMP__LAST_
+ /* Allow a safe subset of non-executable directives. See classification
in
+ array c_omp_directives. */
and
+ case EXEC_OMP_ASSUME:
+ case EXEC_OMP_METADIRECTIVE:
+ /* Per OpenMP 6.0, some non-executable directives are allowed in
+ * intervening code. */
You could also cross ref to the gfc_omp_directives array for symmetry.
* * *
With the nits fixed: LGTM.
Tobias
PS: For Fortran, we really should fix PR122306 - the way we handle it
currently is ugly in multiple ways and, as shown, breaks in some cases.
But that's largely unrelated to your modifications.
