public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795]
@ 2023-12-05  6:55 Jakub Jelinek
  2023-12-05 15:07 ` Jason Merrill
  2023-12-05 15:21 ` [PATCH] c++: " Marek Polacek
  0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2023-12-05  6:55 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi!

When committing the #pragma GCC unroll patch, I found I forgot one spot
for diagnosting the invalid unrolls - if #pragma GCC unroll argument is
dependent and the pragma is before a range for loop, the unroll tree (now,
before one converted form ushort) is saved into RANGE_FOR_UNROLL and
tsubst_stmt was RECURing on it, but didn't diagnose if it was invalid and
so we ICEd later in the middle-end when  ANNOTATE_EXPR had unexpected
argument.

The following patch fixes that.  Or should I create some helper function
(how to call it) and call it from all of cp_parser_pragma_unroll,
tsubst_stmt (here) and tsubst_expr (ANNOTATE_EXPR)?
Another option is diagnose it instead when we create the ANNOTATE_EXPRs,
but unfortunately that is in 3 different places.  And at least for the
non-template case we'd have worse location_t.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2023-12-04  Jakub Jelinek  <jakub@redhat.com>

	PR c++/112795
	* pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Perform RANGE_FOR_UNROLL
	value checking here as well.

	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
	-std=gnu++98.
	* g++.dg/ext/unroll-3.C: Likewise.
	* g++.dg/ext/unroll-7.C: New test.
	* g++.dg/ext/unroll-8.C: New test.

--- gcc/cp/pt.cc.jj	2023-12-04 08:59:06.000000000 +0100
+++ gcc/cp/pt.cc	2023-12-04 10:49:38.149254907 +0100
@@ -18407,22 +18407,46 @@ tsubst_stmt (tree t, tree args, tsubst_f
 					complain, in_decl, decomp);
 	  }
 
+	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
+	if (unroll)
+	  {
+	    HOST_WIDE_INT lunroll;
+	    if (type_dependent_expression_p (unroll))
+	      ;
+	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
+		     || (!value_dependent_expression_p (unroll)
+			 && (!tree_fits_shwi_p (unroll)
+			     || (lunroll = tree_to_shwi (unroll)) < 0
+			     || lunroll >= USHRT_MAX)))
+	      {
+		error_at (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
+			  "%<#pragma GCC unroll%> requires an "
+			  "assignment-expression that evaluates to a "
+			  "non-negative integral constant less than %u",
+			  USHRT_MAX);
+		unroll = integer_one_node;
+	      }
+	    else if (TREE_CODE (unroll) == INTEGER_CST)
+	      {
+		unroll = fold_convert (integer_type_node, unroll);
+		if (integer_zerop (unroll))
+		  unroll = integer_one_node;
+	      }
+	  }
+
 	if (processing_template_decl)
 	  {
 	    RANGE_FOR_IVDEP (stmt) = RANGE_FOR_IVDEP (t);
-	    RANGE_FOR_UNROLL (stmt) = RANGE_FOR_UNROLL (t);
+	    RANGE_FOR_UNROLL (stmt) = unroll;
 	    RANGE_FOR_NOVECTOR (stmt) = RANGE_FOR_NOVECTOR (t);
 	    finish_range_for_decl (stmt, decl, expr);
 	    if (decomp && decl != error_mark_node)
 	      cp_finish_decomp (decl, decomp);
 	  }
 	else
-	  {
-	    tree unroll = RECUR (RANGE_FOR_UNROLL (t));
-	    stmt = cp_convert_range_for (stmt, decl, expr, decomp,
-					 RANGE_FOR_IVDEP (t), unroll,
-					 RANGE_FOR_NOVECTOR (t));
-	  }
+	  stmt = cp_convert_range_for (stmt, decl, expr, decomp,
+				       RANGE_FOR_IVDEP (t), unroll,
+				       RANGE_FOR_NOVECTOR (t));
 
 	bool prev = note_iteration_stmt_body_start ();
         RECUR (RANGE_FOR_BODY (t));
--- gcc/testsuite/g++.dg/ext/unroll-2.C.jj	2020-01-12 11:54:37.172401958 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-2.C	2023-12-04 10:17:00.390997063 +0100
@@ -1,6 +1,5 @@
-// { dg-do compile }
+// { dg-do compile { target c++11 } }
 // { dg-options "-O2 -fdump-tree-cunrolli-details" }
-// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
 
 void
 foo (int (&a)[8], int *b, int *c)
--- gcc/testsuite/g++.dg/ext/unroll-3.C.jj	2020-01-12 11:54:37.172401958 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-3.C	2023-12-04 10:17:13.526813516 +0100
@@ -1,6 +1,5 @@
-// { dg-do compile }
+// { dg-do compile { target c++11 } }
 // { dg-options "-O2 -fdump-tree-cunrolli-details" }
-// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
 
 template <typename T>
 void
--- gcc/testsuite/g++.dg/ext/unroll-7.C.jj	2023-12-04 10:17:53.481255222 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-7.C	2023-12-04 10:39:23.258115349 +0100
@@ -0,0 +1,45 @@
+// PR c++/112795
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -fdump-tree-cunrolli-details" }
+
+void baz (int);
+constexpr int n = 3;
+constexpr int m = 7;
+
+template <typename T>
+void
+foo (int (&a)[3], T b)
+{
+#pragma GCC unroll(n)
+  for (auto i : a)
+    baz (i);
+#pragma GCC unroll(m)
+  for (auto i : b)
+    baz (i);
+}
+
+template <int N>
+void
+bar (int (&a)[N])
+{
+#pragma GCC unroll(N)
+  for (auto i : a)
+    baz (i);
+}
+
+void
+qux ()
+{
+  int a[3] = { 1, 2, 3 };
+  int b[7] = { 4, 5, 6, 7, 8, 9, 10 };
+  int c[6] = { 11, 12, 13, 14, 15, 16 };
+  int d[10] = { 17, 18, 19, 20, 21, 22, 23, 24, 25, 26 };
+  foo <int (&)[7]> (a, b);
+  bar <6> (c);
+  bar <10> (d);
+}
+
+// { dg-final { scan-tree-dump "loop with 3 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 6 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 7 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 10 iterations completely unrolled" "cunrolli" } }
--- gcc/testsuite/g++.dg/ext/unroll-8.C.jj	2023-12-04 10:17:56.763209358 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-8.C	2023-12-04 10:44:58.531217053 +0100
@@ -0,0 +1,86 @@
+// PR c++/112795
+// { dg-do compile { target c++11 } }
+
+void
+foo (int (&a)[3])
+{
+  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+template <int N, typename U>
+void
+bar (U a)
+{
+  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+template <typename T, int N, typename U>
+void
+baz (U a)
+{
+  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N + 0xffffffffffffffffULL)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N - 42)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 1.0f)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 0xffffffffffffffffULL)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) -42)
+  for (auto i : a)
+    ;
+}
+
+template <typename T, int N, typename U>
+void
+qux (U a)
+{
+  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N + 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N - 42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) -42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+void
+corge ()
+{
+  int a[3] = { 1, 2, 3 };
+  qux <float, 0, int (&)[3]> (a);
+}

	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795]
  2023-12-05  6:55 [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795] Jakub Jelinek
@ 2023-12-05 15:07 ` Jason Merrill
  2023-12-05 16:03   ` [PATCH] c++, v2: " Jakub Jelinek
  2023-12-05 15:21 ` [PATCH] c++: " Marek Polacek
  1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2023-12-05 15:07 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 12/5/23 01:55, Jakub Jelinek wrote:
> Hi!
> 
> When committing the #pragma GCC unroll patch, I found I forgot one spot
> for diagnosting the invalid unrolls - if #pragma GCC unroll argument is
> dependent and the pragma is before a range for loop, the unroll tree (now,
> before one converted form ushort) is saved into RANGE_FOR_UNROLL and
> tsubst_stmt was RECURing on it, but didn't diagnose if it was invalid and
> so we ICEd later in the middle-end when  ANNOTATE_EXPR had unexpected
> argument.
> 
> The following patch fixes that.  Or should I create some helper function
> (how to call it) and call it from all of cp_parser_pragma_unroll,
> tsubst_stmt (here) and tsubst_expr (ANNOTATE_EXPR)?

Please.  Maybe check_pragma_unroll? check_unroll_factor?

> Another option is diagnose it instead when we create the ANNOTATE_EXPRs,
> but unfortunately that is in 3 different places.  And at least for the
> non-template case we'd have worse location_t.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2023-12-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/112795
> 	* pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Perform RANGE_FOR_UNROLL
> 	value checking here as well.
> 
> 	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
> 	-std=gnu++98.
> 	* g++.dg/ext/unroll-3.C: Likewise.
> 	* g++.dg/ext/unroll-7.C: New test.
> 	* g++.dg/ext/unroll-8.C: New test.
> 
> --- gcc/cp/pt.cc.jj	2023-12-04 08:59:06.000000000 +0100
> +++ gcc/cp/pt.cc	2023-12-04 10:49:38.149254907 +0100
> @@ -18407,22 +18407,46 @@ tsubst_stmt (tree t, tree args, tsubst_f
>   					complain, in_decl, decomp);
>   	  }
>   
> +	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> +	if (unroll)
> +	  {
> +	    HOST_WIDE_INT lunroll;
> +	    if (type_dependent_expression_p (unroll))
> +	      ;
> +	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> +		     || (!value_dependent_expression_p (unroll)
> +			 && (!tree_fits_shwi_p (unroll)
> +			     || (lunroll = tree_to_shwi (unroll)) < 0
> +			     || lunroll >= USHRT_MAX)))
> +	      {
> +		error_at (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
> +			  "%<#pragma GCC unroll%> requires an "
> +			  "assignment-expression that evaluates to a "
> +			  "non-negative integral constant less than %u",
> +			  USHRT_MAX);
> +		unroll = integer_one_node;
> +	      }
> +	    else if (TREE_CODE (unroll) == INTEGER_CST)
> +	      {
> +		unroll = fold_convert (integer_type_node, unroll);
> +		if (integer_zerop (unroll))
> +		  unroll = integer_one_node;
> +	      }
> +	  }
> +
>   	if (processing_template_decl)
>   	  {
>   	    RANGE_FOR_IVDEP (stmt) = RANGE_FOR_IVDEP (t);
> -	    RANGE_FOR_UNROLL (stmt) = RANGE_FOR_UNROLL (t);
> +	    RANGE_FOR_UNROLL (stmt) = unroll;
>   	    RANGE_FOR_NOVECTOR (stmt) = RANGE_FOR_NOVECTOR (t);
>   	    finish_range_for_decl (stmt, decl, expr);
>   	    if (decomp && decl != error_mark_node)
>   	      cp_finish_decomp (decl, decomp);
>   	  }
>   	else
> -	  {
> -	    tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> -	    stmt = cp_convert_range_for (stmt, decl, expr, decomp,
> -					 RANGE_FOR_IVDEP (t), unroll,
> -					 RANGE_FOR_NOVECTOR (t));
> -	  }
> +	  stmt = cp_convert_range_for (stmt, decl, expr, decomp,
> +				       RANGE_FOR_IVDEP (t), unroll,
> +				       RANGE_FOR_NOVECTOR (t));
>   
>   	bool prev = note_iteration_stmt_body_start ();
>           RECUR (RANGE_FOR_BODY (t));
> --- gcc/testsuite/g++.dg/ext/unroll-2.C.jj	2020-01-12 11:54:37.172401958 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-2.C	2023-12-04 10:17:00.390997063 +0100
> @@ -1,6 +1,5 @@
> -// { dg-do compile }
> +// { dg-do compile { target c++11 } }
>   // { dg-options "-O2 -fdump-tree-cunrolli-details" }
> -// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
>   
>   void
>   foo (int (&a)[8], int *b, int *c)
> --- gcc/testsuite/g++.dg/ext/unroll-3.C.jj	2020-01-12 11:54:37.172401958 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-3.C	2023-12-04 10:17:13.526813516 +0100
> @@ -1,6 +1,5 @@
> -// { dg-do compile }
> +// { dg-do compile { target c++11 } }
>   // { dg-options "-O2 -fdump-tree-cunrolli-details" }
> -// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
>   
>   template <typename T>
>   void
> --- gcc/testsuite/g++.dg/ext/unroll-7.C.jj	2023-12-04 10:17:53.481255222 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-7.C	2023-12-04 10:39:23.258115349 +0100
> @@ -0,0 +1,45 @@
> +// PR c++/112795
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -fdump-tree-cunrolli-details" }
> +
> +void baz (int);
> +constexpr int n = 3;
> +constexpr int m = 7;
> +
> +template <typename T>
> +void
> +foo (int (&a)[3], T b)
> +{
> +#pragma GCC unroll(n)
> +  for (auto i : a)
> +    baz (i);
> +#pragma GCC unroll(m)
> +  for (auto i : b)
> +    baz (i);
> +}
> +
> +template <int N>
> +void
> +bar (int (&a)[N])
> +{
> +#pragma GCC unroll(N)
> +  for (auto i : a)
> +    baz (i);
> +}
> +
> +void
> +qux ()
> +{
> +  int a[3] = { 1, 2, 3 };
> +  int b[7] = { 4, 5, 6, 7, 8, 9, 10 };
> +  int c[6] = { 11, 12, 13, 14, 15, 16 };
> +  int d[10] = { 17, 18, 19, 20, 21, 22, 23, 24, 25, 26 };
> +  foo <int (&)[7]> (a, b);
> +  bar <6> (c);
> +  bar <10> (d);
> +}
> +
> +// { dg-final { scan-tree-dump "loop with 3 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 6 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 7 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 10 iterations completely unrolled" "cunrolli" } }
> --- gcc/testsuite/g++.dg/ext/unroll-8.C.jj	2023-12-04 10:17:56.763209358 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-8.C	2023-12-04 10:44:58.531217053 +0100
> @@ -0,0 +1,86 @@
> +// PR c++/112795
> +// { dg-do compile { target c++11 } }
> +
> +void
> +foo (int (&a)[3])
> +{
> +  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <int N, typename U>
> +void
> +bar (U a)
> +{
> +  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <typename T, int N, typename U>
> +void
> +baz (U a)
> +{
> +  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N + 0xffffffffffffffffULL)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N - 42)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 1.0f)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 0xffffffffffffffffULL)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) -42)
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <typename T, int N, typename U>
> +void
> +qux (U a)
> +{
> +  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N + 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N - 42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) -42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +void
> +corge ()
> +{
> +  int a[3] = { 1, 2, 3 };
> +  qux <float, 0, int (&)[3]> (a);
> +}
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795]
  2023-12-05  6:55 [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795] Jakub Jelinek
  2023-12-05 15:07 ` Jason Merrill
@ 2023-12-05 15:21 ` Marek Polacek
  2023-12-05 15:21   ` Marek Polacek
  1 sibling, 1 reply; 6+ messages in thread
From: Marek Polacek @ 2023-12-05 15:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches

On Tue, Dec 05, 2023 at 07:55:37AM +0100, Jakub Jelinek wrote:
> Hi!
> 
> When committing the #pragma GCC unroll patch, I found I forgot one spot
> for diagnosting the invalid unrolls - if #pragma GCC unroll argument is
> dependent and the pragma is before a range for loop, the unroll tree (now,
> before one converted form ushort) is saved into RANGE_FOR_UNROLL and
> tsubst_stmt was RECURing on it, but didn't diagnose if it was invalid and
> so we ICEd later in the middle-end when  ANNOTATE_EXPR had unexpected
> argument.
> 
> The following patch fixes that.  Or should I create some helper function
> (how to call it) and call it from all of cp_parser_pragma_unroll,
> tsubst_stmt (here) and tsubst_expr (ANNOTATE_EXPR)?
> Another option is diagnose it instead when we create the ANNOTATE_EXPRs,
> but unfortunately that is in 3 different places.  And at least for the
> non-template case we'd have worse location_t.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2023-12-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/112795
> 	* pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Perform RANGE_FOR_UNROLL
> 	value checking here as well.
> 
> 	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
> 	-std=gnu++98.
> 	* g++.dg/ext/unroll-3.C: Likewise.
> 	* g++.dg/ext/unroll-7.C: New test.
> 	* g++.dg/ext/unroll-8.C: New test.
> 
> --- gcc/cp/pt.cc.jj	2023-12-04 08:59:06.000000000 +0100
> +++ gcc/cp/pt.cc	2023-12-04 10:49:38.149254907 +0100
> @@ -18407,22 +18407,46 @@ tsubst_stmt (tree t, tree args, tsubst_f
>  					complain, in_decl, decomp);
>  	  }
>  
> +	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> +	if (unroll)
> +	  {
> +	    HOST_WIDE_INT lunroll;
> +	    if (type_dependent_expression_p (unroll))
> +	      ;
> +	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> +		     || (!value_dependent_expression_p (unroll)
> +			 && (!tree_fits_shwi_p (unroll)
> +			     || (lunroll = tree_to_shwi (unroll)) < 0
> +			     || lunroll >= USHRT_MAX)))
> +	      {
> +		error_at (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
> +			  "%<#pragma GCC unroll%> requires an "
> +			  "assignment-expression that evaluates to a "
> +			  "non-negative integral constant less than %u",
> +			  USHRT_MAX);
> +		unroll = integer_one_node;
> +	      }
> +	    else if (TREE_CODE (unroll) == INTEGER_CST)
> +	      {
> +		unroll = fold_convert (integer_type_node, unroll);
> +		if (integer_zerop (unroll))
> +		  unroll = integer_one_node;
> +	      }
> +	  }

As you note, this is exactly the same code we already have in the
ANNOTATE_EXPR case.  Maybe add check_and_convert_unroll value?

Marek


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795]
  2023-12-05 15:21 ` [PATCH] c++: " Marek Polacek
@ 2023-12-05 15:21   ` Marek Polacek
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Polacek @ 2023-12-05 15:21 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches

On Tue, Dec 05, 2023 at 10:21:18AM -0500, Marek Polacek wrote:
> On Tue, Dec 05, 2023 at 07:55:37AM +0100, Jakub Jelinek wrote:
> > Hi!
> > 
> > When committing the #pragma GCC unroll patch, I found I forgot one spot
> > for diagnosting the invalid unrolls - if #pragma GCC unroll argument is
> > dependent and the pragma is before a range for loop, the unroll tree (now,
> > before one converted form ushort) is saved into RANGE_FOR_UNROLL and
> > tsubst_stmt was RECURing on it, but didn't diagnose if it was invalid and
> > so we ICEd later in the middle-end when  ANNOTATE_EXPR had unexpected
> > argument.
> > 
> > The following patch fixes that.  Or should I create some helper function
> > (how to call it) and call it from all of cp_parser_pragma_unroll,
> > tsubst_stmt (here) and tsubst_expr (ANNOTATE_EXPR)?
> > Another option is diagnose it instead when we create the ANNOTATE_EXPRs,
> > but unfortunately that is in 3 different places.  And at least for the
> > non-template case we'd have worse location_t.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux.
> > 
> > 2023-12-04  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/112795
> > 	* pt.cc (tsubst_stmt) <case RANGE_FOR_STMT>: Perform RANGE_FOR_UNROLL
> > 	value checking here as well.
> > 
> > 	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
> > 	-std=gnu++98.
> > 	* g++.dg/ext/unroll-3.C: Likewise.
> > 	* g++.dg/ext/unroll-7.C: New test.
> > 	* g++.dg/ext/unroll-8.C: New test.
> > 
> > --- gcc/cp/pt.cc.jj	2023-12-04 08:59:06.000000000 +0100
> > +++ gcc/cp/pt.cc	2023-12-04 10:49:38.149254907 +0100
> > @@ -18407,22 +18407,46 @@ tsubst_stmt (tree t, tree args, tsubst_f
> >  					complain, in_decl, decomp);
> >  	  }
> >  
> > +	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> > +	if (unroll)
> > +	  {
> > +	    HOST_WIDE_INT lunroll;
> > +	    if (type_dependent_expression_p (unroll))
> > +	      ;
> > +	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> > +		     || (!value_dependent_expression_p (unroll)
> > +			 && (!tree_fits_shwi_p (unroll)
> > +			     || (lunroll = tree_to_shwi (unroll)) < 0
> > +			     || lunroll >= USHRT_MAX)))
> > +	      {
> > +		error_at (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
> > +			  "%<#pragma GCC unroll%> requires an "
> > +			  "assignment-expression that evaluates to a "
> > +			  "non-negative integral constant less than %u",
> > +			  USHRT_MAX);
> > +		unroll = integer_one_node;
> > +	      }
> > +	    else if (TREE_CODE (unroll) == INTEGER_CST)
> > +	      {
> > +		unroll = fold_convert (integer_type_node, unroll);
> > +		if (integer_zerop (unroll))
> > +		  unroll = integer_one_node;
> > +	      }
> > +	  }
> 
> As you note, this is exactly the same code we already have in the
> ANNOTATE_EXPR case.  Maybe add check_and_convert_unroll value?

I'm too late.  Never mind.

Marek


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] c++, v2: Further #pragma GCC unroll C++ fix [PR112795]
  2023-12-05 15:07 ` Jason Merrill
@ 2023-12-05 16:03   ` Jakub Jelinek
  2023-12-05 16:16     ` Jason Merrill
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-12-05 16:03 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

On Tue, Dec 05, 2023 at 10:07:19AM -0500, Jason Merrill wrote:
> Please.  Maybe check_pragma_unroll? check_unroll_factor?

So like this (assuming it passes bootstrap/regtest, so far passed just
GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 make check-g++ RUNTESTFLAGS="dg.exp='unroll*'"
)?

2023-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/112795
	* cp-tree.h (cp_check_pragma_unroll): Declare.
	* semantics.cc (cp_check_pragma_unroll): New function.
	* parser.cc (cp_parser_pragma_unroll): Use cp_check_pragma_unroll.
	* pt.cc (tsubst_expr) <case ANNOTATE_EXPR>: Likewise.
	(tsubst_stmt) <case RANGE_FOR_STMT>: Likwsie.

	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
	-std=gnu++98.
	* g++.dg/ext/unroll-3.C: Likewise.
	* g++.dg/ext/unroll-7.C: New test.
	* g++.dg/ext/unroll-8.C: New test.

--- gcc/cp/cp-tree.h.jj	2023-12-05 09:06:06.140878013 +0100
+++ gcc/cp/cp-tree.h	2023-12-05 16:21:05.564736203 +0100
@@ -7918,6 +7918,7 @@ extern tree most_general_lambda			(tree)
 extern tree finish_omp_target			(location_t, tree, tree, bool);
 extern void finish_omp_target_clauses		(location_t, tree, tree *);
 extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t);
+extern tree cp_check_pragma_unroll		(location_t, tree);
 
 /* in tree.cc */
 extern int cp_tree_operand_length		(const_tree);
--- gcc/cp/semantics.cc.jj	2023-12-04 08:59:06.888357091 +0100
+++ gcc/cp/semantics.cc	2023-12-05 16:56:03.718410332 +0100
@@ -13016,4 +13016,33 @@ cp_build_bit_cast (location_t loc, tree
   return ret;
 }
 
+/* Diagnose invalid #pragma GCC unroll argument and adjust
+   it if needed.  */
+
+tree
+cp_check_pragma_unroll (location_t loc, tree unroll)
+{
+  HOST_WIDE_INT lunroll = 0;
+  if (type_dependent_expression_p (unroll))
+    ;
+  else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
+	   || (!value_dependent_expression_p (unroll)
+	       && (!tree_fits_shwi_p (unroll)
+		   || (lunroll = tree_to_shwi (unroll)) < 0
+		   || lunroll >= USHRT_MAX)))
+    {
+      error_at (loc, "%<#pragma GCC unroll%> requires an"
+		" assignment-expression that evaluates to a non-negative"
+		" integral constant less than %u", USHRT_MAX);
+      unroll = integer_one_node;
+    }
+  else if (TREE_CODE (unroll) == INTEGER_CST)
+    {
+      unroll = fold_convert (integer_type_node, unroll);
+      if (integer_zerop (unroll))
+	unroll = integer_one_node;
+    }
+  return unroll;
+}
+
 #include "gt-cp-semantics.h"
--- gcc/cp/parser.cc.jj	2023-12-05 09:05:37.533281014 +0100
+++ gcc/cp/parser.cc	2023-12-05 16:18:32.224909370 +0100
@@ -50243,27 +50243,7 @@ cp_parser_pragma_unroll (cp_parser *pars
 {
   location_t location = cp_lexer_peek_token (parser->lexer)->location;
   tree unroll = cp_parser_constant_expression (parser);
-  unroll = fold_non_dependent_expr (unroll);
-  HOST_WIDE_INT lunroll = 0;
-  if (type_dependent_expression_p (unroll))
-    ;
-  else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
-	   || (!value_dependent_expression_p (unroll)
-	       && (!tree_fits_shwi_p (unroll)
-		   || (lunroll = tree_to_shwi (unroll)) < 0
-		   || lunroll >= USHRT_MAX)))
-    {
-      error_at (location, "%<#pragma GCC unroll%> requires an"
-		" assignment-expression that evaluates to a non-negative"
-		" integral constant less than %u", USHRT_MAX);
-      unroll = NULL_TREE;
-    }
-  else if (TREE_CODE (unroll) == INTEGER_CST)
-    {
-      unroll = fold_convert (integer_type_node, unroll);
-      if (integer_zerop (unroll))
-	unroll = integer_one_node;
-    }
+  unroll = cp_check_pragma_unroll (location, fold_non_dependent_expr (unroll));
   cp_parser_skip_to_pragma_eol (parser, pragma_tok);
   return unroll;
 }
--- gcc/cp/pt.cc.jj	2023-12-05 09:06:06.175877520 +0100
+++ gcc/cp/pt.cc	2023-12-05 16:48:05.641109116 +0100
@@ -18407,22 +18407,24 @@ tsubst_stmt (tree t, tree args, tsubst_f
 					complain, in_decl, decomp);
 	  }
 
+	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
+	if (unroll)
+	  unroll
+	    = cp_check_pragma_unroll (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
+				      unroll);
 	if (processing_template_decl)
 	  {
 	    RANGE_FOR_IVDEP (stmt) = RANGE_FOR_IVDEP (t);
-	    RANGE_FOR_UNROLL (stmt) = RANGE_FOR_UNROLL (t);
+	    RANGE_FOR_UNROLL (stmt) = unroll;
 	    RANGE_FOR_NOVECTOR (stmt) = RANGE_FOR_NOVECTOR (t);
 	    finish_range_for_decl (stmt, decl, expr);
 	    if (decomp && decl != error_mark_node)
 	      cp_finish_decomp (decl, decomp);
 	  }
 	else
-	  {
-	    tree unroll = RECUR (RANGE_FOR_UNROLL (t));
-	    stmt = cp_convert_range_for (stmt, decl, expr, decomp,
-					 RANGE_FOR_IVDEP (t), unroll,
-					 RANGE_FOR_NOVECTOR (t));
-	  }
+	  stmt = cp_convert_range_for (stmt, decl, expr, decomp,
+				       RANGE_FOR_IVDEP (t), unroll,
+				       RANGE_FOR_NOVECTOR (t));
 
 	bool prev = note_iteration_stmt_body_start ();
         RECUR (RANGE_FOR_BODY (t));
@@ -21506,30 +21508,8 @@ tsubst_expr (tree t, tree args, tsubst_f
 	tree op3 = RECUR (TREE_OPERAND (t, 2));
 	if (TREE_CODE (op2) == INTEGER_CST
 	    && wi::to_widest (op2) == (int) annot_expr_unroll_kind)
-	  {
-	    HOST_WIDE_INT lunroll;
-	    if (type_dependent_expression_p (op3))
-	      ;
-	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (op3))
-		     || (!value_dependent_expression_p (op3)
-			 && (!tree_fits_shwi_p (op3)
-			     || (lunroll = tree_to_shwi (op3)) < 0
-			     || lunroll >= USHRT_MAX)))
-	      {
-		error_at (EXPR_LOCATION (TREE_OPERAND (t, 2)),
-			  "%<#pragma GCC unroll%> requires an "
-			  "assignment-expression that evaluates to a "
-			  "non-negative integral constant less than %u",
-			  USHRT_MAX);
-		op3 = integer_one_node;
-	      }
-	    else if (TREE_CODE (op3) == INTEGER_CST)
-	      {
-		op3 = fold_convert (integer_type_node, op3);
-		if (integer_zerop (op3))
-		  op3 = integer_one_node;
-	      }
-	  }
+	  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));
       }
--- gcc/testsuite/g++.dg/ext/unroll-2.C.jj	2023-12-04 11:55:52.154142189 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-2.C	2023-12-05 16:17:29.976792279 +0100
@@ -1,6 +1,5 @@
-// { dg-do compile }
+// { dg-do compile { target c++11 } }
 // { dg-options "-O2 -fdump-tree-cunrolli-details" }
-// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
 
 void
 foo (int (&a)[8], int *b, int *c)
--- gcc/testsuite/g++.dg/ext/unroll-3.C.jj	2023-12-04 11:55:52.154142189 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-3.C	2023-12-05 16:17:29.988792108 +0100
@@ -1,6 +1,5 @@
-// { dg-do compile }
+// { dg-do compile { target c++11 } }
 // { dg-options "-O2 -fdump-tree-cunrolli-details" }
-// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
 
 template <typename T>
 void
--- gcc/testsuite/g++.dg/ext/unroll-7.C.jj	2023-12-05 16:17:29.988792108 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-7.C	2023-12-05 16:17:29.988792108 +0100
@@ -0,0 +1,45 @@
+// PR c++/112795
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -fdump-tree-cunrolli-details" }
+
+void baz (int);
+constexpr int n = 3;
+constexpr int m = 7;
+
+template <typename T>
+void
+foo (int (&a)[3], T b)
+{
+#pragma GCC unroll(n)
+  for (auto i : a)
+    baz (i);
+#pragma GCC unroll(m)
+  for (auto i : b)
+    baz (i);
+}
+
+template <int N>
+void
+bar (int (&a)[N])
+{
+#pragma GCC unroll(N)
+  for (auto i : a)
+    baz (i);
+}
+
+void
+qux ()
+{
+  int a[3] = { 1, 2, 3 };
+  int b[7] = { 4, 5, 6, 7, 8, 9, 10 };
+  int c[6] = { 11, 12, 13, 14, 15, 16 };
+  int d[10] = { 17, 18, 19, 20, 21, 22, 23, 24, 25, 26 };
+  foo <int (&)[7]> (a, b);
+  bar <6> (c);
+  bar <10> (d);
+}
+
+// { dg-final { scan-tree-dump "loop with 3 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 6 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 7 iterations completely unrolled" "cunrolli" } }
+// { dg-final { scan-tree-dump "loop with 10 iterations completely unrolled" "cunrolli" } }
--- gcc/testsuite/g++.dg/ext/unroll-8.C.jj	2023-12-05 16:17:29.988792108 +0100
+++ gcc/testsuite/g++.dg/ext/unroll-8.C	2023-12-05 16:17:29.988792108 +0100
@@ -0,0 +1,86 @@
+// PR c++/112795
+// { dg-do compile { target c++11 } }
+
+void
+foo (int (&a)[3])
+{
+  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+template <int N, typename U>
+void
+bar (U a)
+{
+  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+template <typename T, int N, typename U>
+void
+baz (U a)
+{
+  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N + 0xffffffffffffffffULL)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N - 42)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 1.0f)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 0xffffffffffffffffULL)
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) -42)
+  for (auto i : a)
+    ;
+}
+
+template <typename T, int N, typename U>
+void
+qux (U a)
+{
+  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N + 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll (N - 42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+  #pragma GCC unroll ((T) -42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
+  for (auto i : a)
+    ;
+}
+
+void
+corge ()
+{
+  int a[3] = { 1, 2, 3 };
+  qux <float, 0, int (&)[3]> (a);
+}


	Jakub


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] c++, v2: Further #pragma GCC unroll C++ fix [PR112795]
  2023-12-05 16:03   ` [PATCH] c++, v2: " Jakub Jelinek
@ 2023-12-05 16:16     ` Jason Merrill
  0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2023-12-05 16:16 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

On 12/5/23 11:03, Jakub Jelinek wrote:
> On Tue, Dec 05, 2023 at 10:07:19AM -0500, Jason Merrill wrote:
>> Please.  Maybe check_pragma_unroll? check_unroll_factor?
> 
> So like this (assuming it passes bootstrap/regtest, so far passed just
> GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 make check-g++ RUNTESTFLAGS="dg.exp='unroll*'"
> )?

OK.

> 2023-12-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/112795
> 	* cp-tree.h (cp_check_pragma_unroll): Declare.
> 	* semantics.cc (cp_check_pragma_unroll): New function.
> 	* parser.cc (cp_parser_pragma_unroll): Use cp_check_pragma_unroll.
> 	* pt.cc (tsubst_expr) <case ANNOTATE_EXPR>: Likewise.
> 	(tsubst_stmt) <case RANGE_FOR_STMT>: Likwsie.
> 
> 	* g++.dg/ext/unroll-2.C: Use { target c++11 } instead of dg-skip-if for
> 	-std=gnu++98.
> 	* g++.dg/ext/unroll-3.C: Likewise.
> 	* g++.dg/ext/unroll-7.C: New test.
> 	* g++.dg/ext/unroll-8.C: New test.
> 
> --- gcc/cp/cp-tree.h.jj	2023-12-05 09:06:06.140878013 +0100
> +++ gcc/cp/cp-tree.h	2023-12-05 16:21:05.564736203 +0100
> @@ -7918,6 +7918,7 @@ extern tree most_general_lambda			(tree)
>   extern tree finish_omp_target			(location_t, tree, tree, bool);
>   extern void finish_omp_target_clauses		(location_t, tree, tree *);
>   extern void maybe_warn_unparenthesized_assignment (tree, tsubst_flags_t);
> +extern tree cp_check_pragma_unroll		(location_t, tree);
>   
>   /* in tree.cc */
>   extern int cp_tree_operand_length		(const_tree);
> --- gcc/cp/semantics.cc.jj	2023-12-04 08:59:06.888357091 +0100
> +++ gcc/cp/semantics.cc	2023-12-05 16:56:03.718410332 +0100
> @@ -13016,4 +13016,33 @@ cp_build_bit_cast (location_t loc, tree
>     return ret;
>   }
>   
> +/* Diagnose invalid #pragma GCC unroll argument and adjust
> +   it if needed.  */
> +
> +tree
> +cp_check_pragma_unroll (location_t loc, tree unroll)
> +{
> +  HOST_WIDE_INT lunroll = 0;
> +  if (type_dependent_expression_p (unroll))
> +    ;
> +  else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> +	   || (!value_dependent_expression_p (unroll)
> +	       && (!tree_fits_shwi_p (unroll)
> +		   || (lunroll = tree_to_shwi (unroll)) < 0
> +		   || lunroll >= USHRT_MAX)))
> +    {
> +      error_at (loc, "%<#pragma GCC unroll%> requires an"
> +		" assignment-expression that evaluates to a non-negative"
> +		" integral constant less than %u", USHRT_MAX);
> +      unroll = integer_one_node;
> +    }
> +  else if (TREE_CODE (unroll) == INTEGER_CST)
> +    {
> +      unroll = fold_convert (integer_type_node, unroll);
> +      if (integer_zerop (unroll))
> +	unroll = integer_one_node;
> +    }
> +  return unroll;
> +}
> +
>   #include "gt-cp-semantics.h"
> --- gcc/cp/parser.cc.jj	2023-12-05 09:05:37.533281014 +0100
> +++ gcc/cp/parser.cc	2023-12-05 16:18:32.224909370 +0100
> @@ -50243,27 +50243,7 @@ cp_parser_pragma_unroll (cp_parser *pars
>   {
>     location_t location = cp_lexer_peek_token (parser->lexer)->location;
>     tree unroll = cp_parser_constant_expression (parser);
> -  unroll = fold_non_dependent_expr (unroll);
> -  HOST_WIDE_INT lunroll = 0;
> -  if (type_dependent_expression_p (unroll))
> -    ;
> -  else if (!INTEGRAL_TYPE_P (TREE_TYPE (unroll))
> -	   || (!value_dependent_expression_p (unroll)
> -	       && (!tree_fits_shwi_p (unroll)
> -		   || (lunroll = tree_to_shwi (unroll)) < 0
> -		   || lunroll >= USHRT_MAX)))
> -    {
> -      error_at (location, "%<#pragma GCC unroll%> requires an"
> -		" assignment-expression that evaluates to a non-negative"
> -		" integral constant less than %u", USHRT_MAX);
> -      unroll = NULL_TREE;
> -    }
> -  else if (TREE_CODE (unroll) == INTEGER_CST)
> -    {
> -      unroll = fold_convert (integer_type_node, unroll);
> -      if (integer_zerop (unroll))
> -	unroll = integer_one_node;
> -    }
> +  unroll = cp_check_pragma_unroll (location, fold_non_dependent_expr (unroll));
>     cp_parser_skip_to_pragma_eol (parser, pragma_tok);
>     return unroll;
>   }
> --- gcc/cp/pt.cc.jj	2023-12-05 09:06:06.175877520 +0100
> +++ gcc/cp/pt.cc	2023-12-05 16:48:05.641109116 +0100
> @@ -18407,22 +18407,24 @@ tsubst_stmt (tree t, tree args, tsubst_f
>   					complain, in_decl, decomp);
>   	  }
>   
> +	tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> +	if (unroll)
> +	  unroll
> +	    = cp_check_pragma_unroll (EXPR_LOCATION (RANGE_FOR_UNROLL (t)),
> +				      unroll);
>   	if (processing_template_decl)
>   	  {
>   	    RANGE_FOR_IVDEP (stmt) = RANGE_FOR_IVDEP (t);
> -	    RANGE_FOR_UNROLL (stmt) = RANGE_FOR_UNROLL (t);
> +	    RANGE_FOR_UNROLL (stmt) = unroll;
>   	    RANGE_FOR_NOVECTOR (stmt) = RANGE_FOR_NOVECTOR (t);
>   	    finish_range_for_decl (stmt, decl, expr);
>   	    if (decomp && decl != error_mark_node)
>   	      cp_finish_decomp (decl, decomp);
>   	  }
>   	else
> -	  {
> -	    tree unroll = RECUR (RANGE_FOR_UNROLL (t));
> -	    stmt = cp_convert_range_for (stmt, decl, expr, decomp,
> -					 RANGE_FOR_IVDEP (t), unroll,
> -					 RANGE_FOR_NOVECTOR (t));
> -	  }
> +	  stmt = cp_convert_range_for (stmt, decl, expr, decomp,
> +				       RANGE_FOR_IVDEP (t), unroll,
> +				       RANGE_FOR_NOVECTOR (t));
>   
>   	bool prev = note_iteration_stmt_body_start ();
>           RECUR (RANGE_FOR_BODY (t));
> @@ -21506,30 +21508,8 @@ tsubst_expr (tree t, tree args, tsubst_f
>   	tree op3 = RECUR (TREE_OPERAND (t, 2));
>   	if (TREE_CODE (op2) == INTEGER_CST
>   	    && wi::to_widest (op2) == (int) annot_expr_unroll_kind)
> -	  {
> -	    HOST_WIDE_INT lunroll;
> -	    if (type_dependent_expression_p (op3))
> -	      ;
> -	    else if (!INTEGRAL_TYPE_P (TREE_TYPE (op3))
> -		     || (!value_dependent_expression_p (op3)
> -			 && (!tree_fits_shwi_p (op3)
> -			     || (lunroll = tree_to_shwi (op3)) < 0
> -			     || lunroll >= USHRT_MAX)))
> -	      {
> -		error_at (EXPR_LOCATION (TREE_OPERAND (t, 2)),
> -			  "%<#pragma GCC unroll%> requires an "
> -			  "assignment-expression that evaluates to a "
> -			  "non-negative integral constant less than %u",
> -			  USHRT_MAX);
> -		op3 = integer_one_node;
> -	      }
> -	    else if (TREE_CODE (op3) == INTEGER_CST)
> -	      {
> -		op3 = fold_convert (integer_type_node, op3);
> -		if (integer_zerop (op3))
> -		  op3 = integer_one_node;
> -	      }
> -	  }
> +	  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));
>         }
> --- gcc/testsuite/g++.dg/ext/unroll-2.C.jj	2023-12-04 11:55:52.154142189 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-2.C	2023-12-05 16:17:29.976792279 +0100
> @@ -1,6 +1,5 @@
> -// { dg-do compile }
> +// { dg-do compile { target c++11 } }
>   // { dg-options "-O2 -fdump-tree-cunrolli-details" }
> -// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
>   
>   void
>   foo (int (&a)[8], int *b, int *c)
> --- gcc/testsuite/g++.dg/ext/unroll-3.C.jj	2023-12-04 11:55:52.154142189 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-3.C	2023-12-05 16:17:29.988792108 +0100
> @@ -1,6 +1,5 @@
> -// { dg-do compile }
> +// { dg-do compile { target c++11 } }
>   // { dg-options "-O2 -fdump-tree-cunrolli-details" }
> -// { dg-skip-if "range for" { *-*-* } { "-std=gnu++98" } { "" } }
>   
>   template <typename T>
>   void
> --- gcc/testsuite/g++.dg/ext/unroll-7.C.jj	2023-12-05 16:17:29.988792108 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-7.C	2023-12-05 16:17:29.988792108 +0100
> @@ -0,0 +1,45 @@
> +// PR c++/112795
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -fdump-tree-cunrolli-details" }
> +
> +void baz (int);
> +constexpr int n = 3;
> +constexpr int m = 7;
> +
> +template <typename T>
> +void
> +foo (int (&a)[3], T b)
> +{
> +#pragma GCC unroll(n)
> +  for (auto i : a)
> +    baz (i);
> +#pragma GCC unroll(m)
> +  for (auto i : b)
> +    baz (i);
> +}
> +
> +template <int N>
> +void
> +bar (int (&a)[N])
> +{
> +#pragma GCC unroll(N)
> +  for (auto i : a)
> +    baz (i);
> +}
> +
> +void
> +qux ()
> +{
> +  int a[3] = { 1, 2, 3 };
> +  int b[7] = { 4, 5, 6, 7, 8, 9, 10 };
> +  int c[6] = { 11, 12, 13, 14, 15, 16 };
> +  int d[10] = { 17, 18, 19, 20, 21, 22, 23, 24, 25, 26 };
> +  foo <int (&)[7]> (a, b);
> +  bar <6> (c);
> +  bar <10> (d);
> +}
> +
> +// { dg-final { scan-tree-dump "loop with 3 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 6 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 7 iterations completely unrolled" "cunrolli" } }
> +// { dg-final { scan-tree-dump "loop with 10 iterations completely unrolled" "cunrolli" } }
> --- gcc/testsuite/g++.dg/ext/unroll-8.C.jj	2023-12-05 16:17:29.988792108 +0100
> +++ gcc/testsuite/g++.dg/ext/unroll-8.C	2023-12-05 16:17:29.988792108 +0100
> @@ -0,0 +1,86 @@
> +// PR c++/112795
> +// { dg-do compile { target c++11 } }
> +
> +void
> +foo (int (&a)[3])
> +{
> +  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <int N, typename U>
> +void
> +bar (U a)
> +{
> +  #pragma GCC unroll 1.0f			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll 0xffffffffffffffffULL	// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll -42			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <typename T, int N, typename U>
> +void
> +baz (U a)
> +{
> +  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N + 0xffffffffffffffffULL)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N - 42)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 1.0f)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 0xffffffffffffffffULL)
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) -42)
> +  for (auto i : a)
> +    ;
> +}
> +
> +template <typename T, int N, typename U>
> +void
> +qux (U a)
> +{
> +  #pragma GCC unroll (N + 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N + 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll (N - 42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 1.0f)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) 0xffffffffffffffffULL)// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +  #pragma GCC unroll ((T) -42)			// { dg-error "'#pragma GCC unroll' requires an assignment-expression that evaluates to a non-negative integral constant less than" }
> +  for (auto i : a)
> +    ;
> +}
> +
> +void
> +corge ()
> +{
> +  int a[3] = { 1, 2, 3 };
> +  qux <float, 0, int (&)[3]> (a);
> +}
> 
> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-12-05 16:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-05  6:55 [PATCH] c++: Further #pragma GCC unroll C++ fix [PR112795] Jakub Jelinek
2023-12-05 15:07 ` Jason Merrill
2023-12-05 16:03   ` [PATCH] c++, v2: " Jakub Jelinek
2023-12-05 16:16     ` Jason Merrill
2023-12-05 15:21 ` [PATCH] c++: " Marek Polacek
2023-12-05 15:21   ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).