On 27/08/2026 13:25, Tobias Burnus wrote:
Tobias Burnus wrote:
while I still need to continue looking through the patches
(1/3 + this follow up), I thought I could already start by
sending you three ICE examples and two please-add wish for
an existing diagnostic and for missing diagnostic.
* * *
I start first with comments done in 2/3 - the actual example code
parts are handled by this follow-up patch, i.e. [::] / [n::] now
actually work. (I think that was the only C++ applicable part of
those examples.)
* * *
First an RFC:
Additionally, I wonderwhether it wouldn't be cleaner to have an
'enum ... : unsigned char' instead of a bare 'unsigned char' – it adds
some type safety and also makes it easier to see what it is about. The
patch does in c-tree.h:
... and likewise in gcc/cp/parser.h:
+#define OMP_ARRAY_SECTION_NONE 0
+#define OMP_ARRAY_SECTION_UNSTRIDED 1
+#define OMP_ARRAY_SECTION_STRIDED 2
+ unsigned char omp_array_section_kind;
which then gets used as:
@@ -9889 +9960,2 @@ cp_parser_parenthesized_expression_list (cp_parser*
parser,
- bool saved_omp_array_section_p;
+ unsigned char saved_omp_array_section_kind;
besides some 'auto' + 'make_temp_override' use.
In C++, it comes into existence via ggc_cleared_alloc in
cp_parser_new - instead of being a global var as in C.
Albeit thanks to 'make_temp_override' it seems to be less
important. (Although, whatever is chosen, it probably makes
sense to use either an enum or not in both C and C++.)
Yes, mirrored the enum c_omp_array_section_kind to omp_array_section_kind.
* * *
* * *
+cp_omp_create_arrayshape_type (location_t loc, tree expr,
...
+ error ("OpenMP array shaping operator with non-pointer
argument");
As written in the 2/3 patch:
Can we have an 'error_at' here? I guess "EXPR_LOCATION (expr)" will
work. [This also applies to the C++ FE code.]
Done. It was even easier here because cp_omp_create_arrayshape_type
already had the loc parameter.
* * *
* * *
This patch changes:
can if MAYBE_ZERO_LEN is false. MAYBE_ZERO_LEN will be true in
the above
case though, as some lengths could be zero. */
static tree
handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
bool &maybe_zero_len, unsigned int
&first_non_one,
- enum c_omp_region_type ort)
+ enum c_omp_region_type ort, int
*discontiguous)
and
+/* Handle array sections for clause C. On entry *DISCONTIGUOUS is 0
if array
+ section must be contiguous, 1 if it can be discontiguous, and in
the latter
+ case it is set to 2 on exit if it is determined to be
discontiguous during
+ the function's execution. */
static bool
-handle_omp_array_sections (tree &c, enum c_omp_region_type ort)
+handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
+ int *discontiguous = NULL)
However, I think the same applies here as what I wrote to patch 2/3
This is mostly asking for adding argument documentation - possibly
with changing the internal representation as that might be clearer
and makes also writing the documentation easier.
I also would like to see one or two minor changes of code which
is more confusing than helpful and the like.
Full verbatim quote from 2/3, which seems to be fully applicable.
[Sorry, probably should be copy-edited it - as it is partially
backward as I discovered more.]
First and unrelated to this patch, I think it would be helpful to
document
the return value of these two functions (handle_omp_array_sections_1 and
handle_omp_array_sections). Namely,
The aux _1 function returns a tree - the other a Boolean.
For the latter 'true' means an error, the former returns
error_mark_node in case of an error, for non OpenMP array sections
essentially 't' (except for a convert_lvalue_to_rvalue conversion
in a corner case) - and for array sections either build_array_ref or
build_omp_array_section.
* * *
I think the aux function needs also comment documenting the discontiguous
argument.
For the handle_omp_array_sections_1 aux function:
If known to be discontiguous:
* Print an error if discontiguous is a nullptr or *discontiguous == 0
* Otherwise, set *discontiguous = 2
If discontiguous && *discontiguous != 0 - build_omp_array_section.
For handle_omp_array_sections:
If discontiguous && *discontiguous
* When the array might be noncontigous, set *discontiguous = 2
* If at the end *discontiguous remains with value != 2, i.e. the
array section is known to be contiguous, convert the array section
back to an build_array_ref.
Hence:
For the aux function, '2' strictly means that it is known to
be noncontiguous.
For the other function, '2' means that it might be noncontigous,
'1' that it is known to be noncontigous, and '0' that semantic
requires noncontigous (with a compile time check, but still could
go wrong at runtime).
Noncontiguous can be either if stride != 1 [with some exceptions] but
also for multidimension variables for, e.g. 'arr[1:n][0:3]'
if 'int arr[...][size]' and size > 3 [except n == 1].
* * *
I wonder whether it wouldn't be cleaner to make DISCONTIGUOUS
required in the aux function. Using the current argument wording
for handle_omp_array_sections could be reused.
Otherwise, it should mention that nullptr and *discontigous == 0
both mean the same. - The nullptr bit could be removed, if the
pointer argument is alwas passed (like in the variant below):
* * *
Regarding the required part, that's because I wonder whether it is
cleaner to use ...
... in handle_omp_array_sections, the argument 'bool *discontiguous_p'
with:
int discontigous = discontiguous_p && *discontiguous_p ? 1 : 0;
...
// Discontiguous permitted but known to be contiguous.
if (discontigous == 1)
discontigous_p = false;
and passing this 'discontiguous' as argument to the aux function (i.e
it will never be a nullptr in the aux function).
Alternatively, if keeping it as integer, I wonder whether it would
be cleaner to set 'discontiguous' to '0' if known to be known to be
contiguous instead of keeping the old value.
If keeping it as integer: It should be made clear that '2' implies
that it might be discontigous (but it might be only known at runtime)
while the original value (or if set to zero, the value 0) means that
it is known to be contiguous.
And in in either case (bool* or int*), it should be made clear that
a nullptr acts like setting the value to 0 / false.
* * *
If we know that *discontiguous can only be 0 or two, the following
+ if (discontiguous && *discontiguous != 2)
+ first = omp_array_section_low_bound (OMP_CLAUSE_LOCATION (c),
first);
could be replaced by '*discontigous == 1' - as 0 won't create an array
section
and for 2 we want to retain it.
* * *
Actually, I think my claim that _1 only sets it to 2 when it is known to
be noncontiguous is not quite right:
> /* If there is a pointer type anywhere but in the very first
> array-section-subscript, the array section could be non-
contiguous. */
> if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND
> && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_AFFINITY
> && TREE_CODE (TREE_OPERAND (t, 0)) == OMP_ARRAY_SECTION)
> {
> /* If any prior dimension has a non-one length, then deem
this
> array section as non-contiguous. */
> for (tree d = TREE_OPERAND (t, 0);
> TREE_CODE (d) == OMP_ARRAY_SECTION;
> d = TREE_OPERAND (d, 0))
> {
> tree d_length = TREE_OPERAND (d, 2);
> - if (d_length == NULL_TREE || !integer_onep (d_length))
> + tree d_stride = TREE_OPERAND (d, 3);
> + if (d_length == NULL_TREE || !integer_onep (d_length)
> + || (d_stride && !integer_onep (d_stride)))
Assume:
to(arr[:1:m]) or to(arr[:1:2])
Those are the elements:
{ lower-bound, ... , lower-bound + ((length - 1) * stride) }
Namely, only '{ lower-bound }' remains - such that the array section
can still be contiguous.
On the other hand, for:
to(arr[:2]) or to(arr[:n])
the condition is already true - independent whether there is any
stride or not.
(for n == 1 it would be still contiguous - but that's not handled in
this very
special case.)
Thus, I think we can remove the 'd_stride' here - as it either leads
either to
a false positive or is redundant.
* * *
Hence, I think for
> +handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
> ....
> + if (!integer_onep (stride)
> + || (higher_discontiguous
> + && (!integer_zerop (low_bound)
> + || !full_span)))
> + *discontiguous = 2;
> +
> + if (!integer_onep (stride)
> + || !integer_zerop (low_bound)
> + || !full_span)
> + higher_discontiguous = true;
IMHO, it should be for both:
if ((!integer_onep (stride) && !integer_onep (length)
for the same reasons (if length is one, the stride >= 1
does not matter).
* * *
> - if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP)
> + if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP
> + && !(discontiguous && *discontiguous == 2))
> return false;
I wonder whether a comment would help:
// Done, except for MAP clauses and for array sections that may be
noncontiguous
At least I find it otherwise a bit hard to read with the ! (not) and
more complex
conditions - especially as at a glance, there is no real relation
between one and
the other. (Namely: Why aren't TO/FROM handled the same way as map, if
contiguous?
Answer: See longer comment below.)
Side note: c_omp_address_inspector (which is later called) actually
turns the
TO/FROM clause to a MAP clause of map kind GOMP_MAP_{TO,FROM}_GRID;
however,
that happens after this check (and is actually the reason for
continuing).
* * *
Back to the aux function:
> @@ -16198,14 +16277,42 @@ handle_omp_array_sections_1 (tree c, tree
t, vec<tree> &types,
> - ret = build_array_ref (OMP_CLAUSE_LOCATION (c), ret, low_bound);
> + /* NOTE: Stride/length are discarded for affinity/depend here. */
> + if (discontiguous
> + && *discontiguous
> + && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_AFFINITY
> + && OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND)
> + ret = build_omp_array_section (OMP_CLAUSE_LOCATION (c), ret,
low_bound,
> + length, stride);
> + else
> + ret = build_array_ref (OMP_CLAUSE_LOCATION (c), ret, low_bound);
> return ret;
Can we remove the comment and clause-code checks here? Namely:
I have to admit that I find the comment and the check for
affinity/depend more confusing than helpful.
All callers either pass no discontiguous argument (→ NULL default),
except for one:
+ int discontiguous
+ = (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TO
+ || OMP_CLAUSE_CODE (c) == OMP_CLAUSE_FROM);
+ if (handle_omp_array_sections (c, ort, &discontiguous))
And this one explicitly handles those two clauses.
The reason that I find it odd is that the aux function is also called
for a much of other clauses, e.f., OMP_CLAUSE_MAP or OMP_CLAUSE__CACHE_
for which the same applies.
Mostly like in C. One difference though, the
OMP_CLAUSE_AFFINITY/OMP_CLAUSE_DEPEND exclusion has to stay because
finish_omp_clauses also passes `discontiguous = true` for these two clauses.
* * *
* * *
And finally:
+static tree
+omp_noncontig_descriptor_type (location_t loc)
+{
+ static tree cached = NULL_TREE;
+
+ if (cached)
+ return cached;
...
+ tree desc_type = omp_noncontig_descriptor_type
(UNKNOWN_LOCATION);
I think it is cleaner to have no argument to the function
and use
location_t loc = UNKNOWN_LOCATION;
in the function. It seems to be unlikely that we ever want to have a
location
and that this location makes sense, given that the type-declaration is
cached.
Adjusted as suggested.
* * *
Otherwise, I did not spot anything (besides the ICEs report before in
this thread),
i.e. except for the reported issues both the C++ and C patches should
fine - such
that hopefully the next version can just land.
Thanks again to PA for taking care of Julian's patches!
Tobias
PS: Next is the Fortran patch that modifies the used descriptor [ABI]
(and implements
the feature for Fortran, obviously]. There is also a follow up patch
that handles
multi-segment noncontiguous array sections →
https://gcc.gnu.org/pipermail/gcc-patches/2026-August/726503.html
Thanks,
--
PA
From 52cde57d318af788942d17018425c0da9bce8613 Mon Sep 17 00:00:00 2001
From: Paul-Antoine Arras <[email protected]>
Date: Fri, 28 Aug 2026 11:25:26 +0200
Subject: [PATCH] OpenMP: Further fix strided and shaped-array update issues
for C++
This is the second fixup commit for "OpenMP: Support strided and shaped-array
updates for C++", addressing issues found during review of that patch.
gcc/cp/ChangeLog:
* decl.cc (cp_omp_create_arrayshape_type): Take a location_t and
use it in the error_at.
* parser.cc (cp_parser_new): Adjust for omp_array_section_kind
becoming a fixed-underlying-type enum.
(cp_parser_statement_expr): Likewise.
(cp_parser_postfix_expression): Likewise.
(cp_parser_postfix_open_square_expression): Adjust for the
enum; bail out and skip to the closing "]" when the index
expression is erroneous; fix the diagnostics produced for a
malformed "[: :]" section.
(cp_parser_parenthesized_expression_list): Adjust the saved
omp_array_section_kind variable's type for the new enum.
(cp_parser_cast_expression): Adjust for the fixed-underlying-type enum.
(cp_parser_lambda_expression): Adjust the saved
omp_array_section_kind variable's type for the new enum.
(cp_parser_braced_list): Adjust for the fixed-underlying-type enum.
(cp_parser_skip_up_to_closing_square_bracket): Adjust for the
fixed-underlying-type enum.
* parser.h (enum omp_array_section_kind, OMP_ARRAY_SECTION_NONE,
OMP_ARRAY_SECTION_UNSTRIDED, OMP_ARRAY_SECTION_STRIDED): Turn the
array-section-kind macros and the cp_parser::omp_array_section_kind
field into a fixed-underlying-type enum type.
* semantics.cc (handle_omp_array_sections_1): Document the DISCONTIGUOUS
parameter and return value; drop the redundant stride check when
detecting a non-contiguous pointer-typed dimension; simplify now that
DISCONTIGUOUS is always non-null.
(handle_omp_array_sections): Change the DISCONTIGUOUS parameter
from int * to bool *; only treat a non-unit stride as
discontiguous when the length is also not one; add a comment to
the early-return check.
(finish_omp_clauses): Change the local discontiguous variables to
bool.
gcc/ChangeLog:
* omp-low.cc (omp_noncontig_descriptor_type): Drop the now-unused
location_t parameter, using UNKNOWN_LOCATION internally instead.
(scan_sharing_clauses): Update the call to
omp_noncontig_descriptor_type.
gcc/testsuite/ChangeLog:
* g++.dg/gomp/array-section-5.C: Update the expected diagnostics
for the malformed "a2[: :]" section.
* g++.dg/gomp/bad-array-section-12.C: Remove; the diagnostic it
checked is now covered by array-section-2.c for both C and C++.
---
gcc/cp/decl.cc | 3 +-
gcc/cp/parser.cc | 49 +++++++-----
gcc/cp/parser.h | 12 ++-
gcc/cp/semantics.cc | 77 ++++++++++++-------
gcc/omp-low.cc | 5 +-
gcc/testsuite/g++.dg/gomp/array-section-5.C | 6 +-
.../g++.dg/gomp/bad-array-section-12.C | 38 +++------
.../g++.dg/gomp/bad-array-section-13.C | 16 ----
8 files changed, 107 insertions(+), 99 deletions(-)
delete mode 100644 gcc/testsuite/g++.dg/gomp/bad-array-section-13.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 113608a84df..fa2dc3852f0 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -13735,7 +13735,8 @@ cp_omp_create_arrayshape_type (location_t loc, tree expr,
if (TREE_CODE (type) != POINTER_TYPE)
{
- error ("OpenMP array shaping operator with non-pointer argument");
+ error_at (loc,
+ "OpenMP array shaping operator with non-pointer argument");
return error_mark_node;
}
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 982422d9de8..265d84a3706 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -4780,7 +4780,7 @@ cp_parser_new (cp_lexer *lexer)
parser->oacc_routine = NULL;
/* Disallow OpenMP array sections in expressions. */
- parser->omp_array_section_kind = false;
+ parser->omp_array_section_kind = OMP_ARRAY_SECTION_NONE;
/* Disallow OpenMP array-shaping operator in expressions. */
parser->omp_array_shaping_op_p = false;
@@ -5818,7 +5818,8 @@ static cp_expr
cp_parser_statement_expr (cp_parser *parser)
{
cp_token_position start = cp_parser_start_tentative_firewall (parser);
- auto oas = make_temp_override (parser->omp_array_section_kind, false);
+ auto oas = make_temp_override (parser->omp_array_section_kind,
+ OMP_ARRAY_SECTION_NONE);
auto aso = make_temp_override (parser->omp_array_shaping_op_p, false);
/* Consume the '('. */
@@ -8976,7 +8977,7 @@ cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
switch (token->type)
{
case CPP_OPEN_SPLICE:
- if (!parser->omp_array_section_kind)
+ if (parser->omp_array_section_kind == OMP_ARRAY_SECTION_NONE)
goto default_case;
/* Parse '[: length]' array section. */
postfix_expression
@@ -9371,7 +9372,7 @@ cp_parser_postfix_open_square_expression (cp_parser *parser,
parser->greater_than_is_operator_p = true;
saved_colon_corrects_to_scope_p = parser->colon_corrects_to_scope_p;
- if (parser->omp_array_section_kind)
+ if (parser->omp_array_section_kind != OMP_ARRAY_SECTION_NONE)
parser->colon_corrects_to_scope_p = false;
if (open_splice)
@@ -9392,7 +9393,7 @@ cp_parser_postfix_open_square_expression (cp_parser *parser,
{
/* Handle '[::::' and '[::]' in post_colon_parsing. */
}
- else if (!parser->omp_array_section_kind
+ else if (parser->omp_array_section_kind == OMP_ARRAY_SECTION_NONE
|| (cp_lexer_next_token_is_not (parser->lexer, CPP_COLON)
&& cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_SPLICE)))
{
@@ -9469,10 +9470,17 @@ cp_parser_postfix_open_square_expression (cp_parser *parser,
/*warn_comma_p=*/warn_comma_subscript);
}
+ if (index == error_mark_node)
+ {
+ cp_parser_skip_to_closing_square_bracket (parser);
+ return error_mark_node;
+ }
+
post_colon_parsing:
parser->greater_than_is_operator_p = saved_greater_than_is_operator_p;
- if (cxx_dialect >= cxx23 && parser->omp_array_section_kind
+ if (cxx_dialect >= cxx23
+ && parser->omp_array_section_kind != OMP_ARRAY_SECTION_NONE
&& expression_list.get () != NULL
&& vec_safe_length (expression_list) > 1)
{
@@ -9481,7 +9489,7 @@ post_colon_parsing:
index = error_mark_node;
}
- if (parser->omp_array_section_kind
+ if (parser->omp_array_section_kind != OMP_ARRAY_SECTION_NONE
&& (open_splice || cp_lexer_next_token_is (parser->lexer, CPP_COLON)
|| cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_SPLICE)
|| (parser->omp_array_section_kind == OMP_ARRAY_SECTION_STRIDED
@@ -9508,7 +9516,8 @@ post_colon_parsing:
cp_lexer_peek_nth_token (parser->lexer, 2)->type = CPP_SCOPE;
}
else if (cp_lexer_next_token_is_not (parser->lexer, CPP_CLOSE_SQUARE)
- && !close_splice && !double_scope)
+ && !close_splice && !double_scope
+ && cp_lexer_next_token_is_not (parser->lexer, CPP_COLON))
{
if (cxx_dialect >= cxx23)
{
@@ -9533,7 +9542,7 @@ post_colon_parsing:
length = error_mark_node;
}
}
- else if (cp_lexer_next_token_is_not (parser->lexer, CPP_COLON))
+ else
length
= cp_parser_expression (parser, NULL, /*cast_p=*/false,
/*decltype_p=*/false,
@@ -9567,8 +9576,7 @@ post_colon_parsing:
&& cp_lexer_next_token_is (parser->lexer, CPP_CLOSE_SPLICE))
cp_lexer_consume_token (parser->lexer);
else if (!close_splice
- && (parser->omp_array_section_kind
- == OMP_ARRAY_SECTION_UNSTRIDED
+ && (parser->omp_array_section_kind == OMP_ARRAY_SECTION_UNSTRIDED
|| cp_lexer_next_token_is_not (parser->lexer,
CPP_CLOSE_SPLICE)))
cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
@@ -9957,7 +9965,7 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
{
vec<tree, va_gc> *expression_list;
bool saved_greater_than_is_operator_p;
- unsigned char saved_omp_array_section_kind;
+ enum omp_array_section_kind saved_omp_array_section_kind;
bool saved_omp_array_shaping_op_p;
/* Assume all the expressions will be constant. */
@@ -9978,7 +9986,7 @@ cp_parser_parenthesized_expression_list (cp_parser* parser,
saved_omp_array_section_kind = parser->omp_array_section_kind;
saved_omp_array_shaping_op_p = parser->omp_array_shaping_op_p;
- parser->omp_array_section_kind = false;
+ parser->omp_array_section_kind = OMP_ARRAY_SECTION_NONE;
parser->omp_array_shaping_op_p = false;
cp_expr expr (NULL_TREE);
@@ -11635,7 +11643,8 @@ cp_parser_cast_expression (cp_parser *parser, bool address_p, bool cast_p,
else if (parser->omp_array_shaping_op_p
&& cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SQUARE))
{
- auto oas = make_temp_override (parser->omp_array_section_kind, false);
+ auto oas = make_temp_override (parser->omp_array_section_kind,
+ OMP_ARRAY_SECTION_NONE);
auto aso = make_temp_override (parser->omp_array_shaping_op_p, false);
while (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_SQUARE))
@@ -13022,7 +13031,8 @@ cp_parser_lambda_expression (cp_parser* parser,
cp_binding_level* implicit_template_scope = parser->implicit_template_scope;
bool auto_is_implicit_function_template_parm_p
= parser->auto_is_implicit_function_template_parm_p;
- unsigned char saved_omp_array_section_kind = parser->omp_array_section_kind;
+ enum omp_array_section_kind saved_omp_array_section_kind
+ = parser->omp_array_section_kind;
bool saved_omp_array_shaping_op_p = parser->omp_array_shaping_op_p;
bool saved_in_targ = parser->in_template_argument_list_p;
bool saved_in_declarator_p = parser->in_declarator_p;
@@ -13036,7 +13046,7 @@ cp_parser_lambda_expression (cp_parser* parser,
parser->implicit_template_parms = 0;
parser->implicit_template_scope = 0;
parser->auto_is_implicit_function_template_parm_p = false;
- parser->omp_array_section_kind = false;
+ parser->omp_array_section_kind = OMP_ARRAY_SECTION_NONE;
parser->omp_array_shaping_op_p = false;
parser->in_template_argument_list_p = false;
parser->in_declarator_p = false;
@@ -29307,7 +29317,8 @@ cp_parser_braced_list (cp_parser *parser, bool *non_constant_p /*=nullptr*/)
{
tree initializer;
location_t start_loc = cp_lexer_peek_token (parser->lexer)->location;
- auto oas = make_temp_override (parser->omp_array_section_kind, false);
+ auto oas = make_temp_override (parser->omp_array_section_kind,
+ OMP_ARRAY_SECTION_NONE);
auto aso = make_temp_override (parser->omp_array_shaping_op_p, false);
/* Within a brace-enclosed initializer list, a `>' token is always the
@@ -29384,7 +29395,7 @@ cp_parser_skip_up_to_closing_square_bracket (cp_parser *parser)
return false;
case CPP_OPEN_SPLICE:
- if (!parser->omp_array_section_kind)
+ if (parser->omp_array_section_kind == OMP_ARRAY_SECTION_NONE)
break;
/* FALLTHRU */
@@ -29393,7 +29404,7 @@ cp_parser_skip_up_to_closing_square_bracket (cp_parser *parser)
break;
case CPP_CLOSE_SPLICE:
- if (!parser->omp_array_section_kind)
+ if (parser->omp_array_section_kind == OMP_ARRAY_SECTION_NONE)
break;
/* FALLTHRU */
case CPP_CLOSE_SQUARE:
diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index b9be5175e5f..4ab572b6ac0 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -240,6 +240,12 @@ struct GTY(()) omp_begin_declare_variant_map_entry {
tree ctx; /* The context selector associated with the variant. */
};
+enum omp_array_section_kind : unsigned char {
+ OMP_ARRAY_SECTION_NONE = 0,
+ OMP_ARRAY_SECTION_UNSTRIDED = 1,
+ OMP_ARRAY_SECTION_STRIDED = 2
+};
+
/* The cp_parser structure represents the C++ parser. */
struct GTY(()) cp_parser {
@@ -425,10 +431,8 @@ struct GTY(()) cp_parser {
appear. */
bool omp_attrs_forbidden_p;
-#define OMP_ARRAY_SECTION_NONE 0
-#define OMP_ARRAY_SECTION_UNSTRIDED 1
-#define OMP_ARRAY_SECTION_STRIDED 2
- unsigned char omp_array_section_kind;
+ /* The kind of OpenMP array section being parsed, if any. */
+ enum omp_array_section_kind omp_array_section_kind;
/* TRUE if an OpenMP array-shaping operator is allowed. */
bool omp_array_shaping_op_p;
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 5df9d53094c..2d6a2ba6a02 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -6095,7 +6095,7 @@ public:
T current expression (initially OMP_CLAUSE_DECL), which is either
a TREE_LIST for array-section-subscript (TREE_PURPOSE is low-bound
expression if specified, TREE_VALUE length expression if specified,
- TREE_CHAIN is what it has been specified after, or some decl.
+ TREE_CHAIN is what it has been specified after), or some decl.
TYPES vector is populated with array section types, MAYBE_ZERO_LEN
set to true if any of the array-section-subscript could have length
of zero (explicit or implicit), FIRST_NON_ONE is the index of the
@@ -6108,7 +6108,14 @@ public:
<= FIRST_NON_ONE we diagnose non-contiguous arrays if low bound isn't
0 or length isn't the array domain max + 1, for > FIRST_NON_ONE we
can if MAYBE_ZERO_LEN is false. MAYBE_ZERO_LEN will be true in the above
- case though, as some lengths could be zero. */
+ case though, as some lengths could be zero.
+ On entry *DISCONTIGUOUS is 0 if the array section must be contiguous, and
+ non-zero if a discontiguous section is permitted; this function sets
+ *DISCONTIGUOUS to 2 when it determines the section is definitely
+ discontiguous, and otherwise leaves it unchanged.
+ Return the array reference or array-section tree built for T, or T itself
+ (possibly converted to an rvalue) if it is not an array section, or
+ ERROR_MARK_NODE on error. */
static tree
handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
@@ -6479,12 +6486,9 @@ handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
d = TREE_OPERAND (d, 0))
{
tree d_length = TREE_OPERAND (d, 2);
- tree d_stride = TREE_OPERAND (d, 3);
- if (d_length == NULL_TREE
- || !integer_onep (d_length)
- || (d_stride && !integer_onep (d_stride)))
+ if (d_length == NULL_TREE || !integer_onep (d_length))
{
- if (discontiguous && *discontiguous)
+ if (*discontiguous)
*discontiguous = 2;
else
{
@@ -6523,9 +6527,10 @@ handle_omp_array_sections_1 (tree c, tree t, vec<tree> &types,
OMP_CLAUSE_CODE (c) == OMP_CLAUSE_REDUCTION
|| OMP_CLAUSE_CODE (c) == OMP_CLAUSE_IN_REDUCTION
|| OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TASK_REDUCTION);
- /* NOTE: Stride/length are discarded for affinity/depend here. */
- if (discontiguous
- && *discontiguous
+ /* AFFINITY/DEPEND only need an address, not real array-section semantics;
+ stride/length are discarded for them here even when a discontiguous
+ section is permitted. */
+ if (*discontiguous
&& OMP_CLAUSE_CODE (c) != OMP_CLAUSE_AFFINITY
&& OMP_CLAUSE_CODE (c) != OMP_CLAUSE_DEPEND)
ret = grok_omp_array_section (OMP_CLAUSE_LOCATION (c), ret, low_bound,
@@ -6553,26 +6558,31 @@ omp_array_section_low_bound (location_t loc, tree node)
return node;
}
-/* Handle array sections for clause C. On entry *DISCONTIGUOUS is 0 if array
- section must be contiguous, 1 if it can be discontiguous, and in the latter
- case it is set to 2 on exit if it is determined to be discontiguous during
- the function's execution. */
+/* Handle array sections for clause C. DISCONTIGUOUS_P is NULL, or points to
+ a bool that is false, if the array section must be contiguous. If
+ *DISCONTIGUOUS_P is true on entry, a discontiguous section is permitted;
+ on exit it is left true only if the section was determined to be
+ (possibly) discontiguous, and reset to false if it turned out to be
+ contiguous after all. Return true on error. */
static bool
handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
- int *discontiguous = NULL)
+ bool *discontiguous_p = NULL)
{
bool maybe_zero_len = false;
unsigned int first_non_one = 0;
auto_vec<tree, 10> types;
tree *tp = &OMP_CLAUSE_DECL (c);
+ int discontiguous = discontiguous_p && *discontiguous_p ? 1 : 0;
+ if (discontiguous_p)
+ *discontiguous_p = false;
if ((OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
|| OMP_CLAUSE_CODE (c) == OMP_CLAUSE_AFFINITY)
&& OMP_ITERATOR_DECL_P (*tp))
tp = &TREE_VALUE (*tp);
tree first = handle_omp_array_sections_1 (c, *tp, types,
maybe_zero_len, first_non_one,
- ort, discontiguous);
+ ort, &discontiguous);
if (first == error_mark_node)
return true;
if (first == NULL_TREE)
@@ -6645,7 +6655,7 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
if (stride == NULL_TREE)
stride = size_one_node;
- if (discontiguous && *discontiguous)
+ if (discontiguous)
{
/* This condition is similar to the error check below, but
whereas that checks for a definitely-discontiguous array
@@ -6671,13 +6681,19 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
full_span = true;
}
- if (!integer_onep (stride)
+ /* A stride other than one only matters when the length is
+ also other than one; a length-one dimension touches a
+ single element regardless of stride. */
+ bool length_is_one = length != NULL_TREE && integer_onep (length);
+ bool strided = !integer_onep (stride) && !length_is_one;
+
+ if (strided
|| (higher_discontiguous
&& (!integer_zerop (low_bound)
|| !full_span)))
- *discontiguous = 2;
+ discontiguous = 2;
- if (!integer_onep (stride)
+ if (strided
|| !integer_zerop (low_bound)
|| !full_span)
higher_discontiguous = true;
@@ -6701,8 +6717,8 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
if (!tree_int_cst_equal (length, size))
{
is_noncontiguous:
- if (discontiguous && *discontiguous)
- *discontiguous = 2;
+ if (discontiguous)
+ discontiguous = 2;
else
{
error_at (OMP_CLAUSE_LOCATION (c),
@@ -6819,7 +6835,9 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
OMP_CLAUSE_DECL (c) = t;
return false;
}
- if (discontiguous && *discontiguous != 2)
+ if (discontiguous_p)
+ *discontiguous_p = discontiguous == 2;
+ if (discontiguous == 1)
first = omp_array_section_low_bound (OMP_CLAUSE_LOCATION (c),
first);
OMP_CLAUSE_DECL (c) = first;
@@ -6859,8 +6877,9 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
}
}
- if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP
- && !(discontiguous && *discontiguous == 2))
+ /* Done, except for MAP clauses and for array sections that may be
+ noncontiguous. */
+ if (OMP_CLAUSE_CODE (c) != OMP_CLAUSE_MAP && discontiguous != 2)
return false;
/* FIRST represents the first item of data that we are mapping.
@@ -6895,7 +6914,7 @@ handle_omp_array_sections (tree &c, enum c_omp_region_type ort,
&& addr_tokens[0]->u.structure_base_kind == BASE_DECL
&& addr_tokens[1]->type == ACCESS_METHOD
&& omp_access_chain_p (addr_tokens, 1))
- || (discontiguous && *discontiguous == 2))
+ || discontiguous == 2)
c = nc;
return false;
@@ -9393,7 +9412,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
if (TREE_CODE (t) == OMP_ARRAY_SECTION)
{
- int discontiguous = 1;
+ bool discontiguous = true;
if (handle_omp_array_sections (c, ort, &discontiguous))
remove = true;
else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_DEPEND
@@ -9573,7 +9592,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
grp_start_p = pc;
grp_sentinel = OMP_CLAUSE_CHAIN (c);
- int discontiguous
+ bool discontiguous
= (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_TO
|| OMP_CLAUSE_CODE (c) == OMP_CLAUSE_FROM);
if (handle_omp_array_sections (c, ort, &discontiguous))
@@ -9589,7 +9608,7 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
pre-existing "chained access" case, where C is
reassigned only to advance past appended nodes that
stay chained after the original, unmoved clause.) */
- if (discontiguous == 2)
+ if (discontiguous)
*pc = c;
t = OMP_CLAUSE_DECL (c);
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index c23c93c546c..72a504d664e 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -1171,7 +1171,7 @@ fixup_child_record_type (omp_context *ctx)
in sync with libgomp/libgomp.h omp_noncontig_array_desc. */
static tree
-omp_noncontig_descriptor_type (location_t loc)
+omp_noncontig_descriptor_type (void)
{
static tree cached = NULL_TREE;
@@ -1179,6 +1179,7 @@ omp_noncontig_descriptor_type (location_t loc)
return cached;
tree t = make_node (RECORD_TYPE);
+ location_t loc = UNKNOWN_LOCATION;
tree fields = build_decl (loc, FIELD_DECL, get_identifier ("__ndims"),
size_type_node);
@@ -1784,7 +1785,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
&& (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_GRID
|| OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FROM_GRID))
{
- tree desc_type = omp_noncontig_descriptor_type (UNKNOWN_LOCATION);
+ tree desc_type = omp_noncontig_descriptor_type ();
tree bare = decl;
if (TREE_CODE (bare) == VIEW_CONVERT_EXPR)
diff --git a/gcc/testsuite/g++.dg/gomp/array-section-5.C b/gcc/testsuite/g++.dg/gomp/array-section-5.C
index 80075e6dabf..3b8d4a4a456 100644
--- a/gcc/testsuite/g++.dg/gomp/array-section-5.C
+++ b/gcc/testsuite/g++.dg/gomp/array-section-5.C
@@ -21,11 +21,15 @@ void f() {
char c1[4], c2[4], c3[4], c4[4], c5[4];
#pragma omp target enter data map(b) map(to: a1[::]) // { dg-error "52: expected id-expression before '\\\]' token" }
- #pragma omp target enter data map(b) map(to: a2[: :]) // { dg-error "52: expected primary-expression before ':' token" }
#pragma omp target enter data map(b) map(to: a3[ ::]) // { dg-error "53: expected id-expression before '\\\]' token" }
#pragma omp target enter data map(b) map(to: a4[:: ]) // { dg-error "53: expected id-expression before '\\\]' token" }
#pragma omp target enter data map(b) map(to: a5[ :: ]) // { dg-error "54: expected id-expression before '\\\]' token" }
+ #pragma omp target enter data map(b) map(to: a2[: :])
+ // { dg-error "51: expected '\\\]' before ':' token" "" { target *-*-* } .-1 }
+ // { dg-error "51: expected '\\)' before ':' token" "" { target *-*-* } .-2 }
+ // { dg-error "53: expected an OpenMP clause before '\\\]' token" "" { target *-*-* } .-3 }
+
#pragma omp target enter data map(b) map(to: a1[1::])
// { dg-error "51: expected '\\\]' before '::' token" "" { target *-*-* } .-1 }
// { dg-error "51: expected '\\)' before '::' token" "" { target *-*-* } .-2 }
diff --git a/gcc/testsuite/g++.dg/gomp/bad-array-section-12.C b/gcc/testsuite/g++.dg/gomp/bad-array-section-12.C
index 1a5dd470076..f8640907f0e 100644
--- a/gcc/testsuite/g++.dg/gomp/bad-array-section-12.C
+++ b/gcc/testsuite/g++.dg/gomp/bad-array-section-12.C
@@ -1,32 +1,16 @@
// { dg-do compile }
-/* A stride specifier is only valid for "to"/"from" clauses (including on
- "target update"), not for "map" clauses -- not even a "map" clause
- using the "to"/"from" modifier. */
-
-int main ()
+void f()
{
int x[10];
-
-#pragma omp target enter data map(to: x)
-
-#pragma omp target map(x[0:5:2])
- // { dg-error {expected '\]' before ':' token} "" { target *-*-* } .-1 }
- // { dg-error {expected '\)' before ':' token} "" { target *-*-* } .-2 }
- // { dg-error "expected an OpenMP clause before '\\\]' token" "" { target *-*-* } .-3 }
- ;
-
-#pragma omp target enter data map(to: x[0:5:2])
- // { dg-error {expected '\]' before ':' token} "" { target *-*-* } .-1 }
- // { dg-error {expected '\)' before ':' token} "" { target *-*-* } .-2 }
- // { dg-error "expected an OpenMP clause before '\\\]' token" "" { target *-*-* } .-3 }
-
- /* These are fine: "to"/"from" clauses on "target update" do accept a
- stride. */
-#pragma omp target update to(x[0:5:2])
-#pragma omp target update from(x[0:5:2])
-
-#pragma omp target exit data map(release: x)
-
- return 0;
+ int y[10][10][10];
+ #pragma omp target update to(x[0:0:-1])
+ // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
+ // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
+ #pragma omp target update to(x[0:0:0])
+ // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
+ // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
+ #pragma omp target update to(y[0:0:0])
+ // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
+ // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
}
diff --git a/gcc/testsuite/g++.dg/gomp/bad-array-section-13.C b/gcc/testsuite/g++.dg/gomp/bad-array-section-13.C
deleted file mode 100644
index f8640907f0e..00000000000
--- a/gcc/testsuite/g++.dg/gomp/bad-array-section-13.C
+++ /dev/null
@@ -1,16 +0,0 @@
-// { dg-do compile }
-
-void f()
-{
- int x[10];
- int y[10][10][10];
- #pragma omp target update to(x[0:0:-1])
- // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
- // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
- #pragma omp target update to(x[0:0:0])
- // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
- // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
- #pragma omp target update to(y[0:0:0])
- // { dg-error "negative or zero stride in array section in 'to' clause" "" { target *-*-* } .-1 }
- // { dg-error "must contain at least one 'from' or 'to' clauses" "" { target *-*-* } .-2 }
-}
--
2.55.0
