On Sat, Jul 20, 2024 at 02:42:23PM -0600, Sandra Loosemore wrote:
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -263,9 +263,24 @@ struct GTY(()) c_parser {
> otherwise NULL. */
> vec<c_token, va_gc> *in_omp_attribute_pragma;
>
> + /* When in_omp_attribute_pragma is non-null, these fields save the values
> + of the tokens and tokens_avail fields, so that they can be restored
> + after parsing the attribute. Note that parsing the body of a
> + metadirective uses its own save/restore mechanism as those can be
> + nested with or without the attribute pragmas in the body. */
> + c_token * GTY((skip)) save_tokens;
> + unsigned int save_tokens_avail;
The indentation of the above 2 is wrong.
Plus if those members are for metadirective parsing, their names are too
generic.
> +
> /* Set for omp::decl attribute parsing to the decl to which it
> appertains. */
> tree in_omp_decl_attribute;
> +
> + /* Set if we are processing a statement body associated with a
> + metadirective variant. */
> + BOOL_BITFIELD in_metadirective_body : 1;
And the member ordering creates just too much padding.
Pointer, 32-bit int, pointer, 1-bit bitfield, pointer, 32-bit int,
reordering them slightly would get rid of that.
> +
> + vec<tree> * GTY((skip)) metadirective_body_labels;
> + unsigned int metadirective_region_num;
But more importantly, for something parsed really rarely, wouldn't it be
better to just add a single pointer to a new structure that contains
all you need for metadirective parsing?
> + const char *old_name = IDENTIFIER_POINTER (name);
> + char *new_name = (char *) alloca (strlen (old_name) + 32);
char *new_name = XALLOCAVEC (char, strlen (old_name) + 32);
please.
> + sprintf (new_name, "%s_MDR%u", old_name, parser->metadirective_region_num);
> + return get_identifier (new_name);
> +}
> +
> /* Parse a label (C90 6.6.1, C99 6.8.1, C11 6.8.1).
>
> label:
> @@ -7431,6 +7483,9 @@ c_parser_label (c_parser *parser, tree std_attrs)
> gcc_assert (c_parser_next_token_is (parser, CPP_COLON));
> c_parser_consume_token (parser);
> attrs = c_parser_gnu_attributes (parser);
> + if (parser->in_metadirective_body
> + && parser->metadirective_body_labels->contains (name))
> + name = mangle_metadirective_region_label (parser, name);
> tlab = define_label (loc2, name);
> if (tlab)
> {
> @@ -7658,8 +7713,11 @@ c_parser_statement_after_labels (c_parser *parser,
> bool *if_p,
> c_parser_consume_token (parser);
> if (c_parser_next_token_is (parser, CPP_NAME))
> {
> - stmt = c_finish_goto_label (loc,
> - c_parser_peek_token (parser)->value);
> + tree name = c_parser_peek_token (parser)->value;
> + if (parser->in_metadirective_body
> + && parser->metadirective_body_labels->contains (name))
> + name = mangle_metadirective_region_label (parser, name);
> + stmt = c_finish_goto_label (loc, name);
> c_parser_consume_token (parser);
> }
> else if (c_parser_next_token_is (parser, CPP_MULT))
> @@ -14736,6 +14794,10 @@ c_parser_pragma (c_parser *parser, enum
> pragma_context context, bool *if_p)
> c_parser_omp_nothing (parser);
> return false;
>
> + case PRAGMA_OMP_METADIRECTIVE:
> + c_parser_omp_metadirective (parser, if_p);
> + return true;
> +
> case PRAGMA_OMP_ERROR:
> return c_parser_omp_error (parser, context);
>
> @@ -24879,7 +24941,7 @@ c_parser_omp_declare_simd (c_parser *parser, enum
> pragma_context context)
>
> static tree
> c_parser_omp_context_selector (c_parser *parser, enum omp_tss_code set,
> - tree parms)
> + tree parms, bool metadirective_p)
> {
> tree ret = NULL_TREE;
> do
> @@ -25026,12 +25088,18 @@ c_parser_omp_context_selector (c_parser *parser,
> enum omp_tss_code set,
> {
> mark_exp_read (t);
> t = c_fully_fold (t, false, NULL);
> - /* FIXME: this is bogus, both device_num and
> - condition selectors allow arbitrary expressions. */
Not in 5.0, that is a 5.1 feature that hasn't been implemented in the
initial declare variant implementation (and target_device set with
device_num didn't exist there at all).
> - if (!INTEGRAL_TYPE_P (TREE_TYPE (t))
> - || !tree_fits_shwi_p (t))
> - error_at (token->location, "property must be "
> - "constant integer expression");
> + /* FIXME: I believe it is an unimplemented feature rather
> + than a user error to have non-constant expressions
> + inside "declare variant". */
> + if (!metadirective_p
> + && (!INTEGRAL_TYPE_P (TREE_TYPE (t))
> + || !tree_fits_shwi_p (t)))
> + error_at (token->location,
> + "property must be constant integer expression");
> + else if (metadirective_p
> + && !INTEGRAL_TYPE_P (TREE_TYPE (t)))
> + error_at (token->location,
> + "property must be integer expression");
> else
> properties = make_trait_property (NULL_TREE, t,
> properties);
> + /* Remove the selector from further consideration if can be
> + evaluated as a non-match at this point. */
if it can be ?
Jakub
