Re: [PATCH 1/8] OpenMP: lvalue parsing for map/to/from clauses (C++)

[Tobias Burnus <tobias@codesourcery.com>]

On 05.09.23 21:28, Julian Brown wrote:
> This patch supports "lvalue" parsing (or "locator list item type" parsing)
> for several OpenMP clause types for C++, as required for OpenMP 5.0
> and above.  It is based on the version committed to the og13 branch,
> posted here:
>
>    https://gcc.gnu.org/pipermail/gcc-patches/2023-June/623354.html
>
> which in turn was based on the last version posted upstream:
>
>    https://gcc.gnu.org/pipermail/gcc-patches/2022-December/609040.html
>
> This version has mostly just been rebased.

I had a first pass at the patch and didn't spot anything (C++ code wise);
I have some build/rebase issues and one .texi comment that is trivial and
could also be handled together with the 2/8 patch (adding C support).

* * *

Another rebase is required because of:

1 out of 22 hunks FAILED -- saving rejects to file gcc/cp/parser.cc.rej
1 out of 4 hunks FAILED -- saving rejects to file gcc/cp/pt.cc.rej

but those patch-apply fails look trivial to fix.


And during build, I see:
gcc/cp/decl2.cc:643:20: error: ‘build_non_dependent_expr’ was not declared in this scope; did you mean ‘fold_non_dependent_expr’?

For details, see the two commits:
cd0e05b7ac3 c++: remove NON_DEPENDENT_EXPR, part 2
dad311874ac c++: remove NON_DEPENDENT_EXPR, part 1

* * *

When commenting those, even more build issues show up:

../../repos/gcc/gcc/cp/pt.cc:20493:20: error: ‘tsubst_copy’ was not declared in this scope; did you mean ‘tsubst_scope’?
20493 |         tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, complain, in_decl);
       |                    ^~~~~~~~~~~
       |                    tsubst_scope
In file included from ../../repos/gcc/gcc/cp/pt.cc:31:
../../repos/gcc/gcc/cp/pt.cc: In function ‘bool value_dependent_expression_p(tree)’:
../../repos/gcc/gcc/cp/pt.cc:28009:41: error: ‘t’ was not declared in this scope; did you mean ‘tm’?
28009 |         tree op0 = RECUR (TREE_OPERAND (t, 0));
       |                                         ^
../../repos/gcc/gcc/system.h:1178:61: note: in definition of macro ‘CONST_CAST2’
  1178 | #define CONST_CAST2(TOTYPE,FROMTYPE,X) (const_cast<TOTYPE> (X))
       |                                                             ^
../../repos/gcc/gcc/tree.h:1285:31: note: in expansion of macro ‘TREE_OPERAND_CHECK’
  1285 | #define TREE_OPERAND(NODE, I) TREE_OPERAND_CHECK (NODE, I)
       |                               ^~~~~~~~~~~~~~~~~~
../../repos/gcc/gcc/cp/pt.cc:28009:27: note: in expansion of macro ‘TREE_OPERAND’
28009 |         tree op0 = RECUR (TREE_OPERAND (t, 0));
       |                           ^~~~~~~~~~~~
../../repos/gcc/gcc/cp/pt.cc:28009:20: error: ‘RECUR’ was not declared in this scope
28009 |         tree op0 = RECUR (TREE_OPERAND (t, 0));
       |                    ^~~~~
../../repos/gcc/gcc/cp/pt.cc:28012:11: error: ‘RETURN’ was not declared in this scope
28012 |           RETURN (error_mark_node);
       |           ^~~~~~

* * *

BTW: There is OpenMP specification Issue 2618 which is about code like "map(*p = 10)" with the intent to disallow it.
That's in line with the current code which prints a 'sorry' for those.

* * *

libgomp.texi contains:

@item C/C++'s lvalue expressions in @code{to}, @code{from}
       and @code{map} clauses @tab N @tab

I think this can be set to 'P' + 'Only C++'; I think except for questionable code like '*p = 10'
the support is complete, isn't it? If no, 'Initial support for C++, only' could be a comment.

Alternatively, we can fold it into the next patch (which adds C support).

* * *

> 2023-09-05  Julian Brown<julian@codesourcery.com>
>
> gcc/c-family/
>       * c-common.h (c_omp_address_inspector): Remove static from get_origin
>       and maybe_unconvert_ref methods.
>       * c-omp.cc (c_omp_split_clauses): Support OMP_ARRAY_SECTION.
>       (c_omp_address_inspector::map_supported_p): Handle OMP_ARRAY_SECTION.
>       (c_omp_address_inspector::get_origin): Avoid dereferencing possibly
>       NULL type when processing template decls.
>       (c_omp_address_inspector::maybe_unconvert_ref): Likewise.
>
> gcc/cp/
>       * constexpr.cc (potential_consant_expression_1): Handle
>       OMP_ARRAY_SECTION.
>       * cp-tree.h (grok_omp_array_section, build_omp_array_section): Add
>       prototypes.
>       * decl2.cc (grok_omp_array_section): New function.
>       * error.cc (dump_expr): Handle OMP_ARRAY_SECTION.
>       * parser.cc (cp_parser_new): Initialize parser->omp_array_section_p.
>       (cp_parser_statement_expr): Disallow array sections.
>       (cp_parser_postfix_open_square_expression): Support OMP_ARRAY_SECTION
>       parsing.
>       (cp_parser_parenthesized_expression_list, cp_parser_lambda_expression,
>       cp_parser_braced_list): Disallow array sections.
>       (cp_parser_omp_var_list_no_open): Remove ALLOW_DEREF parameter, add
>       MAP_LVALUE in its place.  Support generalised lvalue parsing for
>       OpenMP map, to and from clauses.  Use OMP_ARRAY_SECTION
>       code instead of TREE_LIST to represent OpenMP array sections.
>       (cp_parser_omp_var_list): Remove ALLOW_DEREF parameter, add MAP_LVALUE.
>       Pass to cp_parser_omp_var_list_no_open.
>       (cp_parser_oacc_data_clause): Update call to cp_parser_omp_var_list.
>       (cp_parser_omp_clause_map): Add sk_omp scope around
>       cp_parser_omp_var_list_no_open call.
>       * parser.h (cp_parser): Add omp_array_section_p field.
>       * pt.cc (tsubst, 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.  Handle more types of map
>       expression.
>       * typeck.cc (build_omp_array_section): New function.
>
> gcc/
>       * gimplify.cc (gimplify_expr): Ensure OMP_ARRAY_SECTION has been
>       processed out before gimplification.
>       * tree-pretty-print.cc (dump_generic_node): Support OMP_ARRAY_SECTION.
>       * tree.def (OMP_ARRAY_SECTION): New tree code.
>
> gcc/testsuite/
>       * c-c++-common/gomp/map-6.c: Update expected output.
>       * g++.dg/gomp/array-section-1.C: New test.
>       * g++.dg/gomp/array-section-2.C: New test.
>       * g++.dg/gomp/bad-array-section-1.C: New test.
>       * g++.dg/gomp/bad-array-section-2.C: New test.
>       * g++.dg/gomp/bad-array-section-3.C: New test.
>       * g++.dg/gomp/bad-array-section-4.C: New test.
>       * g++.dg/gomp/bad-array-section-5.C: New test.
>       * g++.dg/gomp/bad-array-section-6.C: New test.
>       * g++.dg/gomp/bad-array-section-7.C: New test.
>       * g++.dg/gomp/bad-array-section-8.C: New test.
>       * g++.dg/gomp/bad-array-section-9.C: New test.
>       * g++.dg/gomp/bad-array-section-10.C: New test.
>       * g++.dg/gomp/bad-array-section-11.C: New test.
>       * g++.dg/gomp/has_device_addr-non-lvalue-1.C: New test.
>       * g++.dg/gomp/pr67522.C: Update expected output.
>       * g++.dg/gomp/ind-base-3.C: New test.
>       * g++.dg/gomp/map-assignment-1.C: New test.
>       * g++.dg/gomp/map-inc-1.C: New test.
>       * g++.dg/gomp/map-lvalue-ref-1.C: New test.
>       * g++.dg/gomp/map-ptrmem-1.C: New test.
>       * g++.dg/gomp/map-ptrmem-2.C: New test.
>       * g++.dg/gomp/map-static-cast-lvalue-1.C: New test.
>       * g++.dg/gomp/map-ternary-1.C: New test.
>       * g++.dg/gomp/member-array-2.C: New test.
>
> libgomp/
>       * testsuite/libgomp.c++/baseptrs-4.C: Remove commented-out cases that
>       now work.
>       * testsuite/libgomp.c++/baseptrs-6.C: New test.
>       * testsuite/libgomp.c++/ind-base-1.C: New test.
>       * testsuite/libgomp.c++/ind-base-2.C: New test.
>       * testsuite/libgomp.c++/lvalue-tofrom-1.C: New test.
>       * testsuite/libgomp.c++/lvalue-tofrom-2.C: New test.
>       * testsuite/libgomp.c++/map-comma-1.C: New test.
>       * testsuite/libgomp.c++/map-rvalue-ref-1.C: New test.
>       * testsuite/libgomp.c++/struct-ref-1.C: New test.
>       * testsuite/libgomp.c-c++-common/array-field-1.c: New test.
>       * testsuite/libgomp.c-c++-common/array-of-struct-1.c: New test.
>       * testsuite/libgomp.c-c++-common/array-of-struct-2.c: New test.

...

Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955