RE: [PATCH 2/2][frontend]: Add novector C pragma

[Tamar Christina <Tamar.Christina@arm.com>]

Ping.

> -----Original Message-----
> From: Tamar Christina <Tamar.Christina@arm.com>
> Sent: Wednesday, July 26, 2023 8:35 PM
> To: Tamar Christina <Tamar.Christina@arm.com>; gcc-patches@gcc.gnu.org
> Cc: nd <nd@arm.com>; joseph@codesourcery.com
> Subject: RE: [PATCH 2/2][frontend]: Add novector C pragma
> 
> Hi, This is a respin of the patch taking in the feedback received from the C++
> part.
> 
> Simultaneously it's also a ping 😊
> 
> ----
> 
> Hi All,
> 
> FORTRAN currently has a pragma NOVECTOR for indicating that vectorization
> should not be applied to a particular loop.
> 
> ICC/ICX also has such a pragma for C and C++ called #pragma novector.
> 
> As part of this patch series I need a way to easily turn off vectorization of
> particular loops, particularly for testsuite reasons.
> 
> This patch proposes a #pragma GCC novector that does the same for C as
> gfortan does for FORTRAN and what ICX/ICX does for C.
> 
> I added only some basic tests here, but the next patch in the series uses this in
> the testsuite in about ~800 tests.
> 
> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> 
> Ok for master?
> 
> Thanks,
> Tamar
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-pragma.h (enum pragma_kind): Add PRAGMA_NOVECTOR.
> 	* c-pragma.cc (init_pragma): Use it.
> 
> gcc/c/ChangeLog:
> 
> 	* c-parser.cc (c_parser_while_statement, c_parser_do_statement,
> 	c_parser_for_statement, c_parser_statement_after_labels,
> 	c_parse_pragma_novector, c_parser_pragma): Wire through novector
> and
> 	default to false.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/vect/vect-novector-pragma.c: New test.
> 
> --- inline copy of patch ---
> 
> diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h index
> 9cc95ab3ee376628dbef2485b84e6008210fa8fc..99cf2e8bd1c05537c1984
> 70f1aaa0a5a9da4e576 100644
> --- a/gcc/c-family/c-pragma.h
> +++ b/gcc/c-family/c-pragma.h
> @@ -87,6 +87,7 @@ enum pragma_kind {
>    PRAGMA_GCC_PCH_PREPROCESS,
>    PRAGMA_IVDEP,
>    PRAGMA_UNROLL,
> +  PRAGMA_NOVECTOR,
> 
>    PRAGMA_FIRST_EXTERNAL
>  };
> diff --git a/gcc/c-family/c-pragma.cc b/gcc/c-family/c-pragma.cc index
> 0d2b333cebbed32423d5dc6fd2a3ac0ce0bf8b94..848a850b8e123ff1c6ae1e
> c4b7f8ccbd599b1a88 100644
> --- a/gcc/c-family/c-pragma.cc
> +++ b/gcc/c-family/c-pragma.cc
> @@ -1862,6 +1862,10 @@ init_pragma (void)
>      cpp_register_deferred_pragma (parse_in, "GCC", "unroll",
> PRAGMA_UNROLL,
>  				  false, false);
> 
> +  if (!flag_preprocess_only)
> +    cpp_register_deferred_pragma (parse_in, "GCC", "novector",
> PRAGMA_NOVECTOR,
> +				  false, false);
> +
>  #ifdef HANDLE_PRAGMA_PACK_WITH_EXPANSION
>    c_register_pragma_with_expansion (0, "pack", handle_pragma_pack);  #else
> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc index
> 24a6eb6e4596f32c477e3f1c3f98b9792f7bc92c..74f3cbb0d61b5f4c0eb300
> 672f495dde3f1517f7 100644
> --- a/gcc/c/c-parser.cc
> +++ b/gcc/c/c-parser.cc
> @@ -1572,9 +1572,11 @@ static tree c_parser_c99_block_statement
> (c_parser *, bool *,
>  					  location_t * = NULL);
>  static void c_parser_if_statement (c_parser *, bool *, vec<tree> *);  static void
> c_parser_switch_statement (c_parser *, bool *); -static void
> c_parser_while_statement (c_parser *, bool, unsigned short, bool *); -static
> void c_parser_do_statement (c_parser *, bool, unsigned short); -static void
> c_parser_for_statement (c_parser *, bool, unsigned short, bool *);
> +static void c_parser_while_statement (c_parser *, bool, unsigned short, bool,
> +				      bool *);
> +static void c_parser_do_statement (c_parser *, bool, unsigned short,
> +bool); static void c_parser_for_statement (c_parser *, bool, unsigned short,
> bool,
> +				    bool *);
>  static tree c_parser_asm_statement (c_parser *);  static tree
> c_parser_asm_operands (c_parser *);  static tree
> c_parser_asm_goto_operands (c_parser *); @@ -6644,13 +6646,13 @@
> c_parser_statement_after_labels (c_parser *parser, bool *if_p,
>  	  c_parser_switch_statement (parser, if_p);
>  	  break;
>  	case RID_WHILE:
> -	  c_parser_while_statement (parser, false, 0, if_p);
> +	  c_parser_while_statement (parser, false, 0, false, if_p);
>  	  break;
>  	case RID_DO:
> -	  c_parser_do_statement (parser, false, 0);
> +	  c_parser_do_statement (parser, false, 0, false);
>  	  break;
>  	case RID_FOR:
> -	  c_parser_for_statement (parser, false, 0, if_p);
> +	  c_parser_for_statement (parser, false, 0, false, if_p);
>  	  break;
>  	case RID_GOTO:
>  	  c_parser_consume_token (parser);
> @@ -7146,7 +7148,7 @@ c_parser_switch_statement (c_parser *parser, bool
> *if_p)
> 
>  static void
>  c_parser_while_statement (c_parser *parser, bool ivdep, unsigned short
> unroll,
> -			  bool *if_p)
> +			  bool novector, bool *if_p)
>  {
>    tree block, cond, body;
>    unsigned char save_in_statement;
> @@ -7168,6 +7170,11 @@ c_parser_while_statement (c_parser *parser,
> bool ivdep, unsigned short unroll,
>  		   build_int_cst (integer_type_node,
>  				  annot_expr_unroll_kind),
>  		   build_int_cst (integer_type_node, unroll));
> +  if (novector && cond != error_mark_node)
> +    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> +		   build_int_cst (integer_type_node,
> +				  annot_expr_no_vector_kind),
> +		   integer_zero_node);
>    save_in_statement = in_statement;
>    in_statement = IN_ITERATION_STMT;
> 
> @@ -7199,7 +7206,8 @@ c_parser_while_statement (c_parser *parser, bool
> ivdep, unsigned short unroll,  */
> 
>  static void
> -c_parser_do_statement (c_parser *parser, bool ivdep, unsigned short unroll)
> +c_parser_do_statement (c_parser *parser, bool ivdep, unsigned short unroll,
> +		       bool novector)
>  {
>    tree block, cond, body;
>    unsigned char save_in_statement;
> @@ -7228,6 +7236,11 @@ c_parser_do_statement (c_parser *parser, bool
> ivdep, unsigned short unroll)
>  		   build_int_cst (integer_type_node,
>  				  annot_expr_unroll_kind),
>   		   build_int_cst (integer_type_node, unroll));
> +  if (novector && cond != error_mark_node)
> +    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> +		   build_int_cst (integer_type_node,
> +				  annot_expr_no_vector_kind),
> +		   integer_zero_node);
>    if (!c_parser_require (parser, CPP_SEMICOLON, "expected %<;%>"))
>      c_parser_skip_to_end_of_block_or_statement (parser);
> 
> @@ -7296,7 +7309,7 @@ c_parser_do_statement (c_parser *parser, bool
> ivdep, unsigned short unroll)
> 
>  static void
>  c_parser_for_statement (c_parser *parser, bool ivdep, unsigned short unroll,
> -			bool *if_p)
> +			bool novector, bool *if_p)
>  {
>    tree block, cond, incr, body;
>    unsigned char save_in_statement;
> @@ -7452,6 +7465,11 @@ c_parser_for_statement (c_parser *parser, bool
> ivdep, unsigned short unroll,
>   			   build_int_cst (integer_type_node,
>  					  annot_expr_unroll_kind),
>  			   build_int_cst (integer_type_node, unroll));
> +	  if (novector && cond != error_mark_node)
> +	    cond = build3 (ANNOTATE_EXPR, TREE_TYPE (cond), cond,
> +			   build_int_cst (integer_type_node,
> +					  annot_expr_no_vector_kind),
> +			   integer_zero_node);
>  	}
>        /* Parse the increment expression (the third expression in a
>  	 for-statement).  In the case of a foreach-statement, this is @@ -
> 13037,6 +13055,16 @@ c_parse_pragma_ivdep (c_parser *parser)
>    return true;
>  }
> 
> +/* Parse a pragma GCC novector.  */
> +
> +static bool
> +c_parse_pragma_novector (c_parser *parser) {
> +  c_parser_consume_pragma (parser);
> +  c_parser_skip_to_pragma_eol (parser);
> +  return true;
> +}
> +
>  /* Parse a pragma GCC unroll.  */
> 
>  static unsigned short
> @@ -13261,38 +13289,51 @@ c_parser_pragma (c_parser *parser, enum
> pragma_context context, bool *if_p)
>      case PRAGMA_OMP_ORDERED:
>        return c_parser_omp_ordered (parser, context, if_p);
> 
> +    case PRAGMA_NOVECTOR:
> +    case PRAGMA_UNROLL:
>      case PRAGMA_IVDEP:
>        {
> -	const bool ivdep = c_parse_pragma_ivdep (parser);
> -	unsigned short unroll;
> -	if (c_parser_peek_token (parser)->pragma_kind ==
> PRAGMA_UNROLL)
> -	  unroll = c_parser_pragma_unroll (parser);
> -	else
> -	  unroll = 0;
> -	if (!c_parser_next_token_is_keyword (parser, RID_FOR)
> -	    && !c_parser_next_token_is_keyword (parser, RID_WHILE)
> -	    && !c_parser_next_token_is_keyword (parser, RID_DO))
> +	bool novector = false;
> +	unsigned short unroll = 0;
> +	bool ivdep = false;
> +
> +	switch (id)
>  	  {
> -	    c_parser_error (parser, "for, while or do statement expected");
> -	    return false;
> +	  case PRAGMA_NOVECTOR:
> +	    novector = c_parse_pragma_novector (parser);
> +	    break;
> +	  case PRAGMA_UNROLL:
> +	    unroll = c_parser_pragma_unroll (parser);
> +	    break;
> +	  case PRAGMA_IVDEP:
> +	    ivdep = c_parse_pragma_ivdep (parser);
> +	    break;
> +	  default:
> +	    gcc_unreachable ();
>  	  }
> -	if (c_parser_next_token_is_keyword (parser, RID_FOR))
> -	  c_parser_for_statement (parser, ivdep, unroll, if_p);
> -	else if (c_parser_next_token_is_keyword (parser, RID_WHILE))
> -	  c_parser_while_statement (parser, ivdep, unroll, if_p);
> -	else
> -	  c_parser_do_statement (parser, ivdep, unroll);
> -      }
> -      return true;
> 
> -    case PRAGMA_UNROLL:
> -      {
> -	unsigned short unroll = c_parser_pragma_unroll (parser);
> -	bool ivdep;
> -	if (c_parser_peek_token (parser)->pragma_kind == PRAGMA_IVDEP)
> -	  ivdep = c_parse_pragma_ivdep (parser);
> -	else
> -	  ivdep = false;
> +	c_token *tok = c_parser_peek_token (parser);
> +	bool has_more = tok->type == CPP_PRAGMA;
> +	while (has_more)
> +	  {
> +	    switch (tok->pragma_kind)
> +	      {
> +	      case PRAGMA_IVDEP:
> +		ivdep = c_parse_pragma_ivdep (parser);
> +		break;
> +	      case PRAGMA_UNROLL:
> +		unroll = c_parser_pragma_unroll (parser);
> +		break;
> +	      case PRAGMA_NOVECTOR:
> +		novector = c_parse_pragma_novector (parser);
> +		break;
> +	      default:
> +		has_more = false;
> +		break;
> +	      }
> +	    tok = c_parser_peek_token (parser);
> +	    has_more = has_more && tok->type == CPP_PRAGMA;
> +	  }
>  	if (!c_parser_next_token_is_keyword (parser, RID_FOR)
>  	    && !c_parser_next_token_is_keyword (parser, RID_WHILE)
>  	    && !c_parser_next_token_is_keyword (parser, RID_DO)) @@ -
> 13301,11 +13342,11 @@ c_parser_pragma (c_parser *parser, enum
> pragma_context context, bool *if_p)
>  	    return false;
>  	  }
>  	if (c_parser_next_token_is_keyword (parser, RID_FOR))
> -	  c_parser_for_statement (parser, ivdep, unroll, if_p);
> +	  c_parser_for_statement (parser, ivdep, unroll, novector, if_p);
>  	else if (c_parser_next_token_is_keyword (parser, RID_WHILE))
> -	  c_parser_while_statement (parser, ivdep, unroll, if_p);
> +	  c_parser_while_statement (parser, ivdep, unroll, novector, if_p);
>  	else
> -	  c_parser_do_statement (parser, ivdep, unroll);
> +	  c_parser_do_statement (parser, ivdep, unroll, novector);
>        }
>        return true;
> 
> diff --git a/gcc/testsuite/gcc.dg/vect/vect-novector-pragma.c
> b/gcc/testsuite/gcc.dg/vect/vect-novector-pragma.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..3c0b8f5d2acbd27628
> 0785e8d0cbe1f7cd650266
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/vect-novector-pragma.c
> @@ -0,0 +1,61 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target vect_int } */
> +
> +void f1 (int * restrict a, int * restrict b, int n) { #pragma GCC
> +novector
> +    for (int i = 0; i < (n & -8); i++)
> +      a[i] += b[i];
> +}
> +
> +void f2 (int * restrict a, int * restrict b, int n) { #pragma GCC
> +novector #pragma GCC ivdep #pragma GCC unroll 2
> +    for (int i = 0; i < (n & -8); i++)
> +      a[i] += b[i];
> +}
> +
> +void f3 (int * restrict a, int * restrict b, int n) { #pragma GCC ivdep
> +#pragma GCC novector #pragma GCC unroll 2
> +    for (int i = 0; i < (n & -8); i++)
> +      a[i] += b[i];
> +}
> +
> +void f4 (int * restrict a, int * restrict b, int n) { #pragma GCC ivdep
> +#pragma GCC unroll 2 #pragma GCC novector
> +    for (int i = 0; i < (n & -8); i++)
> +      a[i] += b[i];
> +}
> +
> +void f5 (int * restrict a, int * restrict b, int n) {
> +    int i = 0;
> +#pragma GCC novector
> +    do
> +      {
> +        a[i] += b[i];
> +        i++;
> +      }
> +    while (i < (n & -8));
> +}
> +
> +void f6 (int * restrict a, int * restrict b, int n) {
> +    int i = 0;
> +#pragma GCC novector
> +    while (i < (n & -8))
> +      {
> +        a[i] += b[i];
> +        i++;
> +      }
> +}
> +
> +/* { dg-final { scan-tree-dump-not "LOOP VECTORIZED" "vect" } } */