Re: [PATCH v2 3/3] p1689r5: initial support
> "Ben" == Ben Boeckel via Gcc-patches writes: Ben> - `-fdeps-file=` specifies the path to the file to write the format to. I don't know how this output is intended to be used, but one mistake made with the other dependency-tracking options was that the output file isn't created atomically. As a consequence, Makefiles normally have to work around this to be robust. If that's a possible issue here then it would be best to handle it in this patch. Tom
Re: [PATCH v2 3/3] p1689r5: initial support
On Tue, Nov 01, 2022 at 08:57:37 -0600, Tom Tromey wrote: > > "Ben" == Ben Boeckel via Gcc-patches writes: > > Ben> - `-fdeps-file=` specifies the path to the file to write the format to. > > I don't know how this output is intended to be used, but one mistake > made with the other dependency-tracking options was that the output file > isn't created atomically. As a consequence, Makefiles normally have to > work around this to be robust. If that's a possible issue here then it > would be best to handle it in this patch. I don't think there'll be any race here because it's the "output" of the rule as far as the build graph is concerned. It's also JSON, so anything reading it "early" will get a partial object and easily detect "something went wrong". And for clarity, the `-o` flag used in CMake with this is just a side effect of the `-E` mechanism used and is completely ignored in the CMake usage of this. --Ben
ABI question: character, value, optional dummy argument [PR107444]
Dear all, I apologize in advance in case I did not do sufficient research, but I need feedback on a question regarding the ABI that does not seem handled by https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html How do we determine if an actual argument corresponding to a character, value, optional dummy argument is present or not? subroutine s (c) character, value, optional :: c end gets translated on x86_64-pc-linux-gnu to void s (character(kind=1)[1:1] c, integer(kind=8) _c) We could: - add a hidden logical argument for the present status as done for other types (this would clearly be an explicit ABI change) - use the passed character length: if it is > 0, then the argument is present, otherwise it is not. (This may require dealing with invalid code, where invocation of the subroutine with a character(len=0) actual argument is either diagnosed as done by the NAG compiler, or treated as undefined behavior.) Thanks, Harald
Re: [PATCH v2 06/11] OpenMP: lvalue parsing for map clauses (C++)
Hi,
On Tue, 24 May 2022 16:15:31 +0200
Jakub Jelinek via Fortran wrote:
> On Fri, Mar 18, 2022 at 09:26:47AM -0700, Julian Brown wrote:
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -4266,6 +4266,9 @@ cp_parser_new (cp_lexer *lexer)
> >parser->omp_declare_simd = NULL;
> >parser->oacc_routine = NULL;
> >
> > + /* Allow array slice in expression. */
>
> Better /* Disallow OpenMP array sections in expressions. */
Fixed.
> > + parser->omp_array_section_p = false;
> > +
> >/* Not declaring an implicit function template. */
> >parser->auto_is_implicit_function_template_parm_p = false;
> >parser->fully_implicit_function_template_p = false;
>
> I think we should figure out when we should temporarily disable
> parser->omp_array_section_p = false;
> and restore it afterwards to a saved value. E.g.
> cp_parser_lambda_expression seems like a good candidate, the fact that
> OpenMP array sections are allowed say in map clause doesn't mean they
> are allowed inside of lambdas and it would be especially hard when
> the lambda is defining a separate function and the search for
> OMP_ARRAY_SECTION probably wouldn't be able to discover those.
> Other spots to consider might be statement expressions, perhaps type
> definitions etc.
I've had a go at doing this -- several expression types now forbid
array-section syntax (see new "bad-array-section-*" tests added). I'm
afraid my C++ isn't quite up to figuring out how it's possible to
define a type inside an expression (inside a map clause) if we forbid
lambdas and statement expressions though -- can you give an example?
> > @@ -8021,6 +8024,7 @@ cp_parser_postfix_open_square_expression
> > (cp_parser *parser, releasing_vec expression_list = NULL;
> >location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> >bool saved_greater_than_is_operator_p;
> > + bool saved_colon_corrects_to_scope_p;
> >
> >/* Consume the `[' token. */
> >cp_lexer_consume_token (parser->lexer);
> > @@ -8028,6 +8032,9 @@ cp_parser_postfix_open_square_expression
> > (cp_parser *parser, saved_greater_than_is_operator_p =
> > parser->greater_than_is_operator_p;
> > parser->greater_than_is_operator_p = true;
> > + saved_colon_corrects_to_scope_p =
> > parser->colon_corrects_to_scope_p;
> > + parser->colon_corrects_to_scope_p = false;
>
> I think the last above line should be guarded on
> if (parser->omp_array_section_p)
> There is no reason to get worse diagnostics in non-OpenMP code or
> even in OpenMP code where array sections aren't allowed.
Fixed.
> > +
> > + /* NOTE: We are reusing using the type of the whole array as
> > the type of
> > +the array section here, which isn't necessarily entirely
> > correct.
> > +Might need revisiting. */
>
> "reusing using" looks weird.
> As for the type of OMP_ARRAY_SECTION trees, perhaps we could
> initially use an incomplete array (so array element would be
> meaningful) and when we figure out the details and the array section
> is contiguous change its type to array type covering it.
This version of the patch makes a best-effort attempt to create an
exact-sized array type at parse time, else falls back to an incomplete
array type if there are e.g. variable bounds. The type is essentially
only used for diagnostics anyway, I think, so that should hopefully be
good enough.
> > + return build3_loc (input_location, OMP_ARRAY_SECTION,
> > +TREE_TYPE (postfix_expression),
> > +postfix_expression, index, length);
> > +}
> > +
> > + parser->colon_corrects_to_scope_p =
> > saved_colon_corrects_to_scope_p; +
> >/* Look for the closing `]'. */
> >cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
> >
> > @@ -36536,7 +36570,7 @@ struct omp_dim
> > static tree
> > cp_parser_omp_var_list_no_open (cp_parser *parser, enum
> > omp_clause_code kind, tree list, bool *colon,
> > - bool allow_deref = false)
> > + bool map_lvalue = false)
> > {
> >auto_vec dims;
> >bool array_section_p;
> > @@ -36547,12 +36581,95 @@ cp_parser_omp_var_list_no_open (cp_parser
> > *parser, enum omp_clause_code kind,
> > parser->colon_corrects_to_scope_p = false; *colon = false;
> > }
> > + begin_scope (sk_omp, NULL);
>
> Why? Base-language-wise, clauses don't introduce a new scope
> for name-lookup.
I think this was in aid of a particular test case
(c-c++-common/gomp/map-6.c) that tests various bad usages of "always"
and "close" modifiers, together with variables called literally
"always" and "close". Parse failures during earlier tests could make
later tests fail without the scope. I've moved the scope-creation to
the appropriate caller. (Is there a better way? Discarding
newly-created symbols on error, perhaps?)
> And if it is really needed, I'd strongly prefer to either do it solely
> for the clauses that might need it, or do begin_scope before fi
[PATCH 2/2] OpenMP: Use OMP_ARRAY_SECTION instead of TREE_LIST in C++ FE
This patch changes the representation of OMP array sections in the
C++ front end to use the new OMP_ARRAY_SECTION tree code instead of a
TREE_LIST. This is important for "declare mapper" support, because the
array section representation may stick around longer (in "declare
mapper" definitions), and special-case handling TREE_LIST becomes
necessary in more places, which starts to become unwieldy.
This patch has been rebased, but is largely unchanged from the last
version posted.
Tested alongside previous patch with offloading to NVPTX. OK?
Thanks,
Julian
2022-11-01 Julian Brown
gcc/c-family/
* c-omp.cc (c_omp_split_clauses): Support OMP_ARRAY_SECTION.
gcc/cp/
* parser.cc (cp_parser_omp_var_list_no_open): Use OMP_ARRAY_SECTION
code instead of TREE_LIST to represent OpenMP array sections.
* pt.cc (tsubst_copy, tsubst_omp_clause_decl, tsubst_copy_and_build):
Add OMP_ARRAY_SECTION support.
* semantics.cc (handle_omp_array_sections_1, handle_omp_array_sections,
cp_oacc_check_attachments, finish_omp_clauses): Use OMP_ARRAY_SECTION
instead of TREE_LIST where appropriate.
* gimplify.cc (gimplify_expr): Ensure OMP_ARRAY_SECTION has been
processed out before gimplification.
>From 21bf4b48a172ca20788d5cba0d686102600b8ed4 Mon Sep 17 00:00:00 2001
From: Julian Brown
Date: Thu, 3 Feb 2022 11:37:17 -0800
Subject: [PATCH 2/2] OpenMP: Use OMP_ARRAY_SECTION instead of TREE_LIST in C++
FE
This patch changes the representation of OMP array sections in the
C++ front end to use the new OMP_ARRAY_SECTION tree code instead of a
TREE_LIST. This is important for "declare mapper" support, because the
array section representation may stick around longer (in "declare mapper"
definitions), and special-case handling TREE_LIST becomes necessary in
more places, which starts to become unwieldy.
This patch has been rebased, but is largely unchanged from the last
version posted.
2022-11-01 Julian Brown
gcc/c-family/
* c-omp.cc (c_omp_split_clauses): Support OMP_ARRAY_SECTION.
gcc/cp/
* parser.cc (cp_parser_omp_var_list_no_open): Use OMP_ARRAY_SECTION
code instead of TREE_LIST to represent OpenMP array sections.
* pt.cc (tsubst_copy, tsubst_omp_clause_decl, tsubst_copy_and_build):
Add OMP_ARRAY_SECTION support.
* semantics.cc (handle_omp_array_sections_1, handle_omp_array_sections,
cp_oacc_check_attachments, finish_omp_clauses): Use OMP_ARRAY_SECTION
instead of TREE_LIST where appropriate.
* gimplify.cc (gimplify_expr): Ensure OMP_ARRAY_SECTION has been
processed out before gimplification.
---
gcc/c-family/c-omp.cc | 14 ++
gcc/cp/parser.cc | 15 ---
gcc/cp/pt.cc | 52
gcc/cp/semantics.cc | 62 ++-
gcc/gimplify.cc | 3 +++
5 files changed, 112 insertions(+), 34 deletions(-)
diff --git a/gcc/c-family/c-omp.cc b/gcc/c-family/c-omp.cc
index 947014e8983..4a6c9c84f83 100644
--- a/gcc/c-family/c-omp.cc
+++ b/gcc/c-family/c-omp.cc
@@ -2679,6 +2679,9 @@ c_omp_split_clauses (location_t loc, enum tree_code code,
}
else if (TREE_CODE (OMP_CLAUSE_DECL (c)) == TREE_LIST)
{
+ /* TODO: This can go away once we transition all uses of
+ TREE_LIST for representing OMP array sections to
+ OMP_ARRAY_SECTION. */
tree t;
for (t = OMP_CLAUSE_DECL (c);
TREE_CODE (t) == TREE_LIST; t = TREE_CHAIN (t))
@@ -2687,6 +2690,17 @@ c_omp_split_clauses (location_t loc, enum tree_code code,
bitmap_clear_bit (&allocate_head, DECL_UID (t));
break;
}
+ else if (TREE_CODE (OMP_CLAUSE_DECL (c)) == OMP_ARRAY_SECTION)
+ {
+ tree t;
+ for (t = OMP_CLAUSE_DECL (c);
+ TREE_CODE (t) == OMP_ARRAY_SECTION;
+ t = TREE_OPERAND (t, 0))
+ ;
+ if (DECL_P (t))
+ bitmap_clear_bit (&allocate_head, DECL_UID (t));
+ break;
+ }
/* FALLTHRU */
case OMP_CLAUSE_PRIVATE:
case OMP_CLAUSE_FIRSTPRIVATE:
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index cf619e88f46..4fe7454fe68 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -36927,11 +36927,14 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
decl = TREE_OPERAND (decl, 0);
for (int i = dims.length () - 1; i >= 0; i--)
- decl = tree_cons (dims[i].low_bound, dims[i].length, decl);
+ decl = build3_loc (input_location, OMP_ARRAY_SECTION,
+ TREE_TYPE (decl), decl, dims[i].low_bound,
+ dims[i].length);
}
else if (TREE_CODE (decl) == INDIRECT_REF)
{
bool ref_p = REFERENCE_REF_P (decl);
+ tree type = TREE_TYPE (decl);
/* Turn *foo into foo[0:1]. */
decl = TREE_OPERAND (decl, 0);
@@ -36943,7 +36946,8 @@ cp_parser_omp_var_list_no_open (cp_parser *parser, enum omp_clause_code kind,
- it's an indirection of a pointer, turn it into
"foo[0:1]". */
if (!ref_p)
- decl =
