On 7/19/23 11:15, Tamar Christina wrote:
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/cp/ChangeLog:

        * cp-tree.def (RANGE_FOR_STMT): Update comment.
        * cp-tree.h (RANGE_FOR_NOVECTOR): New.
        (cp_convert_range_for, finish_while_stmt_cond, finish_do_stmt,
        finish_for_cond): Add novector param.
        * init.cc (build_vec_init): Default novector to false.
        * method.cc (build_comparison_op): Likewise.
        * parser.cc (cp_parser_statement): Likewise.
        (cp_parser_for, cp_parser_c_for, cp_parser_range_for,
        cp_convert_range_for, cp_parser_iteration_statement,
        cp_parser_omp_for_loop, cp_parser_pragma): Support novector.
        (cp_parser_pragma_novector): New.
        * pt.cc (tsubst_expr): Likewise.
        * semantics.cc (finish_while_stmt_cond, finish_do_stmt,
        finish_for_cond): Likewise.

gcc/ChangeLog:

        * doc/extend.texi: Document it.

gcc/testsuite/ChangeLog:

        * g++.dg/vect/vect.exp (support vect- prefix).
        * g++.dg/vect/vect-novector-pragma.cc: New test.

--- inline copy of patch --
diff --git a/gcc/cp/cp-tree.def b/gcc/cp/cp-tree.def
index 
0e66ca70e00caa1dc4beada1024ace32954e2aaf..c13c8ea98a523c4ef1c55a11e02d5da9db7e367e
 100644
--- a/gcc/cp/cp-tree.def
+++ b/gcc/cp/cp-tree.def
@@ -305,8 +305,8 @@ DEFTREECODE (IF_STMT, "if_stmt", tcc_statement, 4)
/* Used to represent a range-based `for' statement. The operands are
     RANGE_FOR_DECL, RANGE_FOR_EXPR, RANGE_FOR_BODY, RANGE_FOR_SCOPE,
-   RANGE_FOR_UNROLL, and RANGE_FOR_INIT_STMT, respectively.  Only used in
-   templates.  */
+   RANGE_FOR_UNROLL, RANGE_FOR_NOVECTOR and RANGE_FOR_INIT_STMT,
+   respectively.  Only used in templates.  */

This change is unnecessary; RANGE_FOR_NOVECTOR is a flag, not an operand.

  DEFTREECODE (RANGE_FOR_STMT, "range_for_stmt", tcc_statement, 6)
/* Used to represent an expression statement. Use `EXPR_STMT_EXPR' to
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 
dd3665c8ccf48a8a0b1ba2c06400fe50999ea240..8776e8f4cf8266ee715c3e7f943602fdb1acaf79
 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13658,7 +13660,13 @@ cp_parser_c_for (cp_parser *parser, tree scope, tree 
init, bool ivdep,
                       "%<GCC unroll%> pragma");
        condition = error_mark_node;
      }
-  finish_for_cond (condition, stmt, ivdep, unroll);
+  else if (novector)
+    {
+      cp_parser_error (parser, "missing loop condition in loop with "
+                      "%<GCC novector%> pragma");
+      condition = error_mark_node;
+    }

Why is it a problem for a loop with novector to have no condition? This error makes sense for the other pragmas that want to optimize based on the condition, it seems unneeded for this pragma.

+
+       cp_token *tok = pragma_tok;
+
+       do
          {
-           tok = cp_lexer_consume_token (parser->lexer);
-           ivdep = cp_parser_pragma_ivdep (parser, tok);
-           tok = cp_lexer_peek_token (the_parser->lexer);
+           switch (cp_parser_pragma_kind (tok))
+             {
+               case PRAGMA_IVDEP:
+                 {
+                   if (tok != pragma_tok)
+                     tok = cp_lexer_consume_token (parser->lexer);
+                   ivdep = cp_parser_pragma_ivdep (parser, tok);
+                   tok = cp_lexer_peek_token (the_parser->lexer);
+                   break;
+                 }
+               case PRAGMA_UNROLL:
+                 {
+                   if (tok != pragma_tok)
+                     tok = cp_lexer_consume_token (parser->lexer);
+                   unroll = cp_parser_pragma_unroll (parser, tok);
+                   tok = cp_lexer_peek_token (the_parser->lexer);
+                   break;
+                 }
+               case PRAGMA_NOVECTOR:
+                 {
+                   if (tok != pragma_tok)
+                     tok = cp_lexer_consume_token (parser->lexer);
+                   novector = cp_parser_pragma_novector (parser, tok);
+                   tok = cp_lexer_peek_token (the_parser->lexer);
+                   break;
+                 }
+               default:
+                 gcc_unreachable ();

This unreachable seems to assert that if a pragma follows one of these pragmas, it must be another one of these pragmas? That seems wrong; instead of hitting gcc_unreachable() in that case we should fall through to the diagnostic below.

I also think you could factor the peek out of the switch.

diff --git a/gcc/testsuite/g++.dg/vect/vect-novector-pragma.cc 
b/gcc/testsuite/g++.dg/vect/vect-novector-pragma.cc
new file mode 100644
index 
0000000000000000000000000000000000000000..cd5fb7ba9d4806f4d5e484ec68c707ea0e28ad7c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/vect-novector-pragma.cc
@@ -0,0 +1,68 @@
+/* { dg-skip-if "incorrect syntax for c++98" { *-*-* } { "-std=c++98" } { "" } 
} */
+/* { dg-do compile } */

This is usually expressed as { dg-do compile { target c++11 } }

Or you could run in C++98 mode but guard the range-for tests with
#if __cpp_range_based_for

Jason

Reply via email to