* [PATCH][c++ frontend]: check for missing condition for novector [PR115623]
@ 2024-06-25 8:01 Tamar Christina
2024-06-25 16:10 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Tamar Christina @ 2024-06-25 8:01 UTC (permalink / raw)
To: gcc-patches; +Cc: nd, jason, nathan
[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]
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?
Thanks,
Tamar
gcc/cp/ChangeLog:
PR c++/115623
* parser.cc (cp_parser_c_for): Add check for C++ cond.
gcc/testsuite/ChangeLog:
PR c++/115623
* g++.dg/vect/vect-novector-pragma_2.cc: New test.
---
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index e7409b856f1127e303c6515a3bb2d61a10e7c378..24d7b0e4992fdff69951ac5955f304e473f53374 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14107,6 +14107,12 @@ cp_parser_c_for (cp_parser *parser, tree scope, tree init, bool ivdep,
"%<GCC unroll%> pragma");
condition = error_mark_node;
}
+ else if (novector)
+ {
+ cp_parser_error (parser, "missing loop condition in loop with "
+ "%<GCC novector%> pragma");
+ condition = error_mark_node;
+ }
finish_for_cond (condition, stmt, ivdep, unroll, novector);
/* Look for the `;'. */
cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
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..05dba4db1c6544bc53cd05482d1b2e767052cf43
--- /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;
+}
+
+/* { dg-error "missing loop condition in loop with 'GCC novector' pragma before ';' token" "" { target *-*-* } 6 } */
--
[-- Attachment #2: rb18582.patch --]
[-- Type: text/x-diff, Size: 1251 bytes --]
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index e7409b856f1127e303c6515a3bb2d61a10e7c378..24d7b0e4992fdff69951ac5955f304e473f53374 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -14107,6 +14107,12 @@ cp_parser_c_for (cp_parser *parser, tree scope, tree init, bool ivdep,
"%<GCC unroll%> pragma");
condition = error_mark_node;
}
+ else if (novector)
+ {
+ cp_parser_error (parser, "missing loop condition in loop with "
+ "%<GCC novector%> pragma");
+ condition = error_mark_node;
+ }
finish_for_cond (condition, stmt, ivdep, unroll, novector);
/* Look for the `;'. */
cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
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..05dba4db1c6544bc53cd05482d1b2e767052cf43
--- /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;
+}
+
+/* { dg-error "missing loop condition in loop with 'GCC novector' pragma before ';' token" "" { target *-*-* } 6 } */
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][c++ frontend]: check for missing condition for novector [PR115623]
2024-06-25 8:01 [PATCH][c++ frontend]: check for missing condition for novector [PR115623] Tamar Christina
@ 2024-06-25 16:10 ` Jason Merrill
2024-06-25 16:52 ` Tamar Christina
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2024-06-25 16:10 UTC (permalink / raw)
To: Tamar Christina, gcc-patches; +Cc: nd, nathan
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?
> Thanks,
> Tamar
>
> gcc/cp/ChangeLog:
>
> PR c++/115623
> * parser.cc (cp_parser_c_for): Add check for C++ cond.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/115623
> * g++.dg/vect/vect-novector-pragma_2.cc: New test.
>
> ---
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index e7409b856f1127e303c6515a3bb2d61a10e7c378..24d7b0e4992fdff69951ac5955f304e473f53374 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -14107,6 +14107,12 @@ cp_parser_c_for (cp_parser *parser, tree scope, tree init, bool ivdep,
> "%<GCC unroll%> pragma");
> condition = error_mark_node;
> }
> + else if (novector)
> + {
> + cp_parser_error (parser, "missing loop condition in loop with "
> + "%<GCC novector%> pragma");
> + condition = error_mark_node;
> + }
> finish_for_cond (condition, stmt, ivdep, unroll, novector);
> /* Look for the `;'. */
> cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
> 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..05dba4db1c6544bc53cd05482d1b2e767052cf43
> --- /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;
> +}
> +
> +/* { dg-error "missing loop condition in loop with 'GCC novector' pragma before ';' token" "" { target *-*-* } 6 } */
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][c++ frontend]: check for missing condition for novector [PR115623]
2024-06-25 16:10 ` Jason Merrill
@ 2024-06-25 16:52 ` Tamar Christina
2024-06-25 21:23 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Tamar Christina @ 2024-06-25 16:52 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, nd, nathan
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?
Cheers,
Tamar
> > Thanks,
> > Tamar
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/115623
> > * parser.cc (cp_parser_c_for): Add check for C++ cond.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/115623
> > * g++.dg/vect/vect-novector-pragma_2.cc: New test.
> >
> > ---
> > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> > index e7409b856f1127e303c6515a3bb2d61a10e7c378..24d7b0e4992fdff69951ac5955f304e473f53374 100644
> > --- a/gcc/cp/parser.cc
> > +++ b/gcc/cp/parser.cc
> > @@ -14107,6 +14107,12 @@ cp_parser_c_for (cp_parser *parser, tree scope, tree init, bool ivdep,
> > "%<GCC unroll%> pragma");
> > condition = error_mark_node;
> > }
> > + else if (novector)
> > + {
> > + cp_parser_error (parser, "missing loop condition in loop with "
> > + "%<GCC novector%> pragma");
> > + condition = error_mark_node;
> > + }
> > finish_for_cond (condition, stmt, ivdep, unroll, novector);
> > /* Look for the `;'. */
> > cp_parser_require (parser, CPP_SEMICOLON, RT_SEMICOLON);
> > 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..05dba4db1c6544bc53cd05482d1b2e767052cf43
> > --- /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;
> > +}
> > +
> > +/* { dg-error "missing loop condition in loop with 'GCC novector' pragma before ';' token" "" { target *-*-* } 6 } */
> >
> >
> >
> >
>
--
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][c++ frontend]: check for missing condition for novector [PR115623]
2024-06-25 16:52 ` Tamar Christina
@ 2024-06-25 21:23 ` Jason Merrill
2024-06-27 15:25 ` Tamar Christina
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2024-06-25 21:23 UTC (permalink / raw)
To: Tamar Christina; +Cc: gcc-patches, nd, nathan
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.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH][c++ frontend]: check for missing condition for novector [PR115623]
2024-06-25 21:23 ` Jason Merrill
@ 2024-06-27 15:25 ` Tamar Christina
0 siblings, 0 replies; 5+ messages in thread
From: Tamar Christina @ 2024-06-27 15:25 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, nd, nathan
[-- Attachment #1: Type: text/plain, Size: 3209 bytes --]
> -----Original Message-----
> From: Jason Merrill <jason@redhat.com>
> Sent: Tuesday, June 25, 2024 10:24 PM
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: gcc-patches@gcc.gnu.org; nd <nd@arm.com>; nathan@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
[-- Attachment #2: rb18582.patch --]
[-- Type: application/octet-stream, Size: 1030 bytes --]
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;
+}
+
+
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-27 15:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-25 8:01 [PATCH][c++ frontend]: check for missing condition for novector [PR115623] Tamar Christina
2024-06-25 16:10 ` Jason Merrill
2024-06-25 16:52 ` Tamar Christina
2024-06-25 21:23 ` Jason Merrill
2024-06-27 15:25 ` Tamar Christina
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).