On Thu, Nov 23, 2023 at 03:21:41PM +0100, Tobias Burnus wrote: > I stumbled over this trivial omission which blocks some testcases. > > I am not sure whether I have solved the is-same-expr most elegantly, > but I did loosely follow the duplicated-entry check for 'map'. As that's > a restriction to the user, we don't have to catch all and I hope the code > catches the most important violations, doesn't ICE and does not reject > valid code. At least for all real-world code it should™ work, but I > guess for lvalue expressions involving function calls it probably doesn't. > > Thoughts, comments? > > Tobias > > PS: GCC accepts an lvalue expression in C/C++ and only a identifier > for a scalar variable in Fortran, i.e. neither array elements nor > structure components. > > Which variant is right depends whether one reads OpenMP 5.1 (lvalue expr, > scalar variable) or 5.2 (variable without permitting array sections or > structure components) - whereas TR12 has the same but talks about > locator list items in one restriction. For the OpenMP mess, see spec > issue #3739. > ----------------- > 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
> OpenMP: Accept argument to depobj's destroy clause > > Since OpenMP 5.2, the destroy clause takes an depend argument as argument; > for the depobj directive, it the new argument is optional but, if present, > it must be identical to the directive's argument. > > gcc/c/ChangeLog: > > * c-parser.cc (c_parser_omp_depobj): Accept optionally an argument > to the destroy clause. > > gcc/cp/ChangeLog: > > * parser.cc (cp_parser_omp_depobj): Accept optionally an argument > to the destroy clause. > > gcc/fortran/ChangeLog: > > * openmp.cc (gfc_match_omp_depobj): Accept optionally an argument > to the destroy clause. > > libgomp/ChangeLog: > > * libgomp.texi (5.2 Impl. Status): An argument to the destroy clause > is now supported. > > gcc/testsuite/ChangeLog: > > * c-c++-common/gomp/depobj-3.c: New test. > * gfortran.dg/gomp/depobj-3.f90: New test. > > gcc/c/c-parser.cc | 57 ++++++++++++++++++++++++++- > gcc/cp/parser.cc | 60 > ++++++++++++++++++++++++++++- > gcc/fortran/openmp.cc | 15 +++++++- > gcc/testsuite/c-c++-common/gomp/depobj-3.c | 40 +++++++++++++++++++ > gcc/testsuite/gfortran.dg/gomp/depobj-3.f90 | 18 +++++++++ > libgomp/libgomp.texi | 2 +- > 6 files changed, 188 insertions(+), 4 deletions(-) > > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc > index 371dd29557b..378647c1a67 100644 > --- a/gcc/c/c-parser.cc > +++ b/gcc/c/c-parser.cc > @@ -21605,6 +21605,9 @@ c_parser_omp_critical (location_t loc, c_parser > *parser, bool *if_p) > destroy > update (dependence-type) > > + OpenMP 5.2 additionally: > + destroy ( depobj ) > + > dependence-type: > in > out > @@ -21663,7 +21666,59 @@ c_parser_omp_depobj (c_parser *parser) > clause = error_mark_node; > } > else if (!strcmp ("destroy", p)) > - kind = OMP_CLAUSE_DEPEND_LAST; > + { > + matching_parens c_parens; > + kind = OMP_CLAUSE_DEPEND_LAST; > + if (c_parser_next_token_is (parser, CPP_OPEN_PAREN) > + && c_parens.require_open (parser)) > + { > + tree destobj = c_parser_expr_no_commas (parser, NULL).value; > + /* OpenMP requires that the two expressions are identical; catch > + the most common mismatches. */ > + if (!lvalue_p (destobj)) > + error_at (EXPR_LOC_OR_LOC (destobj, c_loc), > + "%<destrory%> expression is not lvalue expression"); > + else if (depobj != error_mark_node) > + { > + tree t = depobj; > + tree t2 = build_unary_op (EXPR_LOC_OR_LOC (destobj, c_loc), > + ADDR_EXPR, destobj, false); > + if (t2 != error_mark_node) > + t2 = build_indirect_ref (EXPR_LOC_OR_LOC (t2, c_loc), > + t2, RO_UNARY_STAR); Please watch indentation, seems there is a mix of 8 spaces vs. tabs: > + while (TREE_CODE (t) == COMPONENT_REF > + || TREE_CODE (t) == ARRAY_REF) > + { > + t = TREE_OPERAND (t, 0); > + if (TREE_CODE (t) == MEM_REF || INDIRECT_REF_P (t)) > + { > + t = TREE_OPERAND (t, 0); > + STRIP_NOPS (t); > + if (TREE_CODE (t) == POINTER_PLUS_EXPR) > + t = TREE_OPERAND (t, 0); > + } > + } > + while (TREE_CODE (t2) == COMPONENT_REF > + || TREE_CODE (t2) == ARRAY_REF) > + { > + t2 = TREE_OPERAND (t2, 0); > + if (TREE_CODE (t2) == MEM_REF || INDIRECT_REF_P (t2)) > + { > + t2 = TREE_OPERAND (t2, 0); > + STRIP_NOPS (t2); > + if (TREE_CODE (t2) == POINTER_PLUS_EXPR) > + t2 = TREE_OPERAND (t2, 0); > + } > + } > + if (DECL_UID (t) != DECL_UID (t2)) Nothing checks that t and t2 here are decls. Use operand_equal_p instead? > + error_at (EXPR_LOC_OR_LOC (destobj, c_loc), > + "the %<destroy%> expression %qE must be the same " > + "as the %<depobj%> argument %qE", > + destobj, depobj); > + } > + c_parens.skip_until_found_close (parser); > + } > + } > + if (t != t2) And here too? Jakub