Hi!

The following testcase ICEs starting with the r14-4229 PR111529
change which moved ANNOTATE_EXPR handling from tsubst_expr to
tsubst_copy_and_build.
ANNOTATE_EXPR is only allowed in the IL to wrap a loop condition,
and the loop condition of while/for loops can be a COMPOUND_EXPR
with DECL_EXPR in the first operand and the corresponding VAR_DECL
in the second, as created by finish_cond
      else if (!empty_expr_stmt_p (cond))
        expr = build2 (COMPOUND_EXPR, TREE_TYPE (expr), cond, expr);
Since then Patrick reworked the instantiation, so that we have now
tsubst_stmt and tsubst_expr and ANNOTATE_EXPR ended up in the latter,
while only tsubst_stmt can handle DECL_EXPR.

Now, the reason why the while/for loops with variable declaration
in the condition works in templates without the pragmas (i.e. without
ANNOTATE_EXPR) is that both the FOR_STMT and WHILE_STMT handling uses
RECUR aka tsubst_stmt in handling of the *_COND operand:
    case FOR_STMT:
      stmt = begin_for_stmt (NULL_TREE, NULL_TREE);
      RECUR (FOR_INIT_STMT (t));
      finish_init_stmt (stmt);
      tmp = RECUR (FOR_COND (t));
      finish_for_cond (tmp, stmt, false, 0, false);
and
    case WHILE_STMT:
      stmt = begin_while_stmt ();
      tmp = RECUR (WHILE_COND (t));
      finish_while_stmt_cond (tmp, stmt, false, 0, false);
Therefore, it will handle DECL_EXPR embedded in COMPOUND_EXPR of the
{WHILE,FOR}_COND just fine.
But if that COMPOUND_EXPR with DECL_EXPR is wrapped with one or more
ANNOTATE_EXPRs, because ANNOTATE_EXPR is now done solely in tsubst_expr
and uses RECUR there (i.e. tsubst_expr), it will ICE on DECL_EXPR in there.

Here are 2 possible fixes for this.
The first one keeps ANNOTATE_EXPR handling in tsubst_expr but uses
tsubst_stmt for the first operand.
The second one moves ANNOTATE_EXPR handling to tsubst_stmt (and uses
tsubst_expr for the second/third operand (it could just RECUR too if you
prefer that)).
Yet another possibility could be to duplicate the ANNOTATE_EXPR handling
from tsubst_expr to tsubst_stmt, where both would just RECUR on its
operands, so if one arrives to ANNOTATE_EXPR from tsubst_stmt, it will
tsubst_stmt recursively, if from tsubst_expr (when?) then it would handle
it using tsubst_expr.

So far just lightly tested (but g++.dg/ext/unroll-4.C and the new test
both pass with both versions of the patch), what do you prefer?  I'd like
to avoid testing too many variants...

2024-04-10  Jakub Jelinek  <ja...@redhat.com>

        PR c++/114409
        * pt.cc (tsubst_expr) <case ANNOTATE_EXPR>: Use tsubst_stmt rather
        than tsubst_expr aka RECUR on op1.

        * g++.dg/ext/pr114409-2.C: New test.

--- gcc/cp/pt.cc.jj     2024-04-09 09:29:04.721521726 +0200
+++ gcc/cp/pt.cc        2024-04-10 14:38:43.591554947 +0200
@@ -21774,7 +21774,10 @@ tsubst_expr (tree t, tree args, tsubst_f
 
     case ANNOTATE_EXPR:
       {
-       op1 = RECUR (TREE_OPERAND (t, 0));
+       /* ANNOTATE_EXPR should only appear in WHILE_COND, DO_COND or
+          FOR_COND expressions, which are tsubsted using tsubst_stmt
+          rather than tsubst_expr and can contain DECL_EXPRs.  */
+       op1 = tsubst_stmt (TREE_OPERAND (t, 0), args, complain, in_decl);
        tree op2 = RECUR (TREE_OPERAND (t, 1));
        tree op3 = RECUR (TREE_OPERAND (t, 2));
        if (TREE_CODE (op2) == INTEGER_CST
--- gcc/testsuite/g++.dg/ext/pr114409-2.C.jj    2024-04-10 14:35:19.693300552 
+0200
+++ gcc/testsuite/g++.dg/ext/pr114409-2.C       2024-04-10 14:35:13.513383766 
+0200
@@ -0,0 +1,36 @@
+// PR c++/114409
+// { dg-do compile }
+// { dg-options "-O2" }
+
+template <typename T>
+T
+foo (T)
+{
+  static T t;
+  return 42 - ++t;
+}
+
+template <typename T>
+void
+bar (T x)
+{
+  #pragma GCC novector
+  while (T y = foo (x))
+    ++y;
+}
+
+template <typename T>
+void
+baz (T x)
+{
+  #pragma GCC novector
+  for (; T y = foo (x); )
+    ++y;
+}
+
+void
+qux ()
+{
+  bar (0);
+  baz (0);
+}

        Jakub
2024-04-10  Jakub Jelinek  <ja...@redhat.com>

        PR c++/114409
        * pt.cc (tsubst_expr) <case ANNOTATE_EXPR>: Move to ...
        (tsubst_stmt) <case ANNOTATE_EXPR>: ... here.  Use tsubst_expr
        instead of RECUR for the last 2 arguments.

        * g++.dg/ext/pr114409-2.C: New test.

--- gcc/cp/pt.cc.jj     2024-04-09 09:29:04.721521726 +0200
+++ gcc/cp/pt.cc        2024-04-10 14:45:25.527142692 +0200
@@ -19433,6 +19433,23 @@ tsubst_stmt (tree t, tree args, tsubst_f
     case PREDICT_EXPR:
       RETURN (add_stmt (copy_node (t)));
 
+    case ANNOTATE_EXPR:
+      {
+       /* Although ANNOTATE_EXPR is an expression, it can only appear in
+          WHILE_COND, DO_COND or FOR_COND expressions, which are tsubsted
+          using tsubst_stmt rather than tsubst_expr and can contain
+          DECL_EXPRs.  */
+       tree op1 = RECUR (TREE_OPERAND (t, 0));
+       tree op2 = tsubst_expr (TREE_OPERAND (t, 1), args, complain, in_decl);
+       tree op3 = tsubst_expr (TREE_OPERAND (t, 2), args, complain, in_decl);
+       if (TREE_CODE (op2) == INTEGER_CST
+           && wi::to_widest (op2) == (int) annot_expr_unroll_kind)
+         op3 = cp_check_pragma_unroll (EXPR_LOCATION (TREE_OPERAND (t, 2)),
+                                       op3);
+       RETURN (build3_loc (EXPR_LOCATION (t), ANNOTATE_EXPR,
+                           TREE_TYPE (op1), op1, op2, op3));
+      }
+
     default:
       gcc_assert (!STATEMENT_CODE_P (TREE_CODE (t)));
 
@@ -21772,19 +21789,6 @@ tsubst_expr (tree t, tree args, tsubst_f
        RETURN (op);
       }
 
-    case ANNOTATE_EXPR:
-      {
-       op1 = RECUR (TREE_OPERAND (t, 0));
-       tree op2 = RECUR (TREE_OPERAND (t, 1));
-       tree op3 = RECUR (TREE_OPERAND (t, 2));
-       if (TREE_CODE (op2) == INTEGER_CST
-           && wi::to_widest (op2) == (int) annot_expr_unroll_kind)
-         op3 = cp_check_pragma_unroll (EXPR_LOCATION (TREE_OPERAND (t, 2)),
-                                       op3);
-       RETURN (build3_loc (EXPR_LOCATION (t), ANNOTATE_EXPR,
-                           TREE_TYPE (op1), op1, op2, op3));
-      }
-
     default:
       /* Handle Objective-C++ constructs, if appropriate.  */
       if (tree subst = objcp_tsubst_expr (t, args, complain, in_decl))
--- gcc/testsuite/g++.dg/ext/pr114409-2.C.jj    2024-04-10 14:35:19.693300552 
+0200
+++ gcc/testsuite/g++.dg/ext/pr114409-2.C       2024-04-10 14:35:13.513383766 
+0200
@@ -0,0 +1,36 @@
+// PR c++/114409
+// { dg-do compile }
+// { dg-options "-O2" }
+
+template <typename T>
+T
+foo (T)
+{
+  static T t;
+  return 42 - ++t;
+}
+
+template <typename T>
+void
+bar (T x)
+{
+  #pragma GCC novector
+  while (T y = foo (x))
+    ++y;
+}
+
+template <typename T>
+void
+baz (T x)
+{
+  #pragma GCC novector
+  for (; T y = foo (x); )
+    ++y;
+}
+
+void
+qux ()
+{
+  bar (0);
+  baz (0);
+}

Reply via email to