RE: [PATCH][c++ frontend]: check for missing condition for novector [PR115623]

[Tamar Christina]

> -----Original Message-----
> From: Jason Merrill <ja...@redhat.com>
> Sent: Tuesday, June 25, 2024 10:24 PM
> To: Tamar Christina <tamar.christ...@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <n...@arm.com>; nat...@acm.org
> Subject: Re: [PATCH][c++ frontend]: check for missing condition for novector
> [PR115623]
> 
> On 6/25/24 12:52, Tamar Christina wrote:
> > The 06/25/2024 17:10, Jason Merrill wrote:
> >> On 6/25/24 04:01, Tamar Christina wrote:
> >>> Hi All,
> >>>
> >>> It looks like I forgot to check in the C++ frontend if a condition exist 
> >>> for the
> >>> loop being adorned with novector.  This causes a segfault because cond 
> >>> isn't
> >>> expected to be null.
> >>>
> >>> This fixes it by issuing the same kind of diagnostics we issue for the 
> >>> other
> >>> pragmas.
> >>>
> >>> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >>>
> >>> Ok for master? and backport to GCC-14?
> >>
> >> Hmm, I'm not sure we want to error in this case; it's pointless, but
> >> indeed we aren't going to vectorize a loop that always loops.  I'd think
> >> we should treat it the same as an explicit "true" condition.  And
> >> perhaps the same for unroll/ivdep.
> >>
> >> Does the C front-end treat the null condition different from a constant
> >> true condition?
> >>
> >
> > No, in the C front-end we error for ivdep and unroll, but for novector we 
> > explicitly
> > suppress it by checking for novector && cond && cond != error_mark_node
> instead of
> > just novector && cond != error_mark_node in the use site.
> >
> > Do you want to handle it that way to be consistent?
> 
> Please.
> 

How about this version:

This fixes it by issuing ignoring the pragma when there's no loop condition
the same way we do in the C frontend.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master? and backport to GCC-14?

Thanks,
Tamar

gcc/cp/ChangeLog:

        PR c++/115623
        * semantics.cc (finish_for_cond): Add check for C++ cond.

gcc/testsuite/ChangeLog:

        PR c++/115623
        * g++.dg/vect/vect-novector-pragma_2.cc: New test.

-- inline copy of patch --

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 
08f5f245e7d11a76b975bb04c0075ded1b3ca8ba..4e1374c98130247eb10e3fe7571fec00834e9c05
 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -1501,7 +1501,7 @@ finish_for_cond (tree cond, tree for_stmt, bool ivdep, 
tree unroll,
                                  build_int_cst (integer_type_node,
                                                 annot_expr_unroll_kind),
                                  unroll);
-  if (novector && cond != error_mark_node)
+  if (novector && cond && cond != error_mark_node)
     FOR_COND (for_stmt) = build3 (ANNOTATE_EXPR,
                                  TREE_TYPE (FOR_COND (for_stmt)),
                                  FOR_COND (for_stmt),
diff --git a/gcc/testsuite/g++.dg/vect/vect-novector-pragma_2.cc 
b/gcc/testsuite/g++.dg/vect/vect-novector-pragma_2.cc
new file mode 100644
index 
0000000000000000000000000000000000000000..d2a8eee8d716188880281b4e34a694576b6783f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/vect-novector-pragma_2.cc
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+
+void f (char *a, int i)
+{
+#pragma GCC novector
+  for (;;i++)
+    a[i] *= 2;
+}
+

> Jason

rb18582.patch
Description: rb18582.patch