* [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
@ 2024-02-21 3:06 Andrew Pinski
2024-03-07 7:01 ` Andrew Pinski (QUIC)
2024-04-30 18:53 ` Jason Merrill
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Pinski @ 2024-02-21 3:06 UTC (permalink / raw)
To: gcc-patches; +Cc: Andrew Pinski
After r7-987-gf17a223de829cb, the access for the elements of a vector type would lose the qualifiers.
So if we had `constvector[0]`, the type of the element of the array would not have const on it.
This was due to a missing build_qualified_type for the inner type of the vector when building the array type.
We need to add back the call to build_qualified_type and now the access has the correct qualifiers. So the
overloads and even if it is a lvalue or rvalue is correctly done.
Note we correctly now reject the testcase gcc.dg/pr83415.c which was incorrectly accepted after r7-987-gf17a223de829cb.
Built and tested for aarch64-linux-gnu.
PR c++/89224
gcc/c-family/ChangeLog:
* c-common.cc (convert_vector_to_array_for_subscript): Call build_qualified_type
for the inner type.
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_array_reference): Compare main variants
for the vector/array types instead of the types directly.
gcc/testsuite/ChangeLog:
* g++.dg/torture/vector-subaccess-1.C: New test.
* gcc.dg/pr83415.c: Change warning to error.
Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
gcc/c-family/c-common.cc | 7 +++++-
gcc/cp/constexpr.cc | 3 ++-
.../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
gcc/testsuite/gcc.dg/pr83415.c | 2 +-
4 files changed, 32 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index e15eff698df..884dd9043f9 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
if (gnu_vector_type_p (TREE_TYPE (*vecp)))
{
tree type = TREE_TYPE (*vecp);
+ tree newitype;
ret = !lvalue_p (*vecp);
@@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
for function parameters. */
c_common_mark_addressable_vec (*vecp);
+ /* Make sure qualifiers are copied from the vector type to the new element
+ of the array type. */
+ newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
+
*vecp = build1 (VIEW_CONVERT_EXPR,
- build_array_type_nelts (TREE_TYPE (type),
+ build_array_type_nelts (newitype,
TYPE_VECTOR_SUBPARTS (type)),
*vecp);
}
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index fa346fe01c9..1fe91d16e8e 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
if (!lval
&& TREE_CODE (ary) == VIEW_CONVERT_EXPR
&& VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
- && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
+ && TYPE_MAIN_VARIANT (TREE_TYPE (t))
+ == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))
ary = TREE_OPERAND (ary, 0);
tree oldidx = TREE_OPERAND (t, 1);
diff --git a/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C b/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
new file mode 100644
index 00000000000..0c8958a4e03
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
@@ -0,0 +1,23 @@
+/* PR c++/89224 */
+
+/* The access of `vector[i]` has the same qualifiers as the original
+ vector which was missing. */
+
+typedef __attribute__((vector_size(16))) unsigned char Int8x8_t;
+
+template <class T>
+void g(T &x) {
+ __builtin_abort();
+}
+template <class T>
+void g(const T &x) {
+ __builtin_exit(0);
+}
+void f(const Int8x8_t x) {
+ g(x[0]);
+}
+int main(void)
+{
+ Int8x8_t x ={};
+ f(x);
+}
diff --git a/gcc/testsuite/gcc.dg/pr83415.c b/gcc/testsuite/gcc.dg/pr83415.c
index 5934c16d97c..2fc85031505 100644
--- a/gcc/testsuite/gcc.dg/pr83415.c
+++ b/gcc/testsuite/gcc.dg/pr83415.c
@@ -7,6 +7,6 @@ int
main (int argc, short *argv[])
{
int i = argc;
- y[i] = 7 - i; /* { dg-warning "read-only" } */
+ y[i] = 7 - i; /* { dg-error "read-only" } */
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
2024-02-21 3:06 [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224] Andrew Pinski
@ 2024-03-07 7:01 ` Andrew Pinski (QUIC)
2024-04-30 18:53 ` Jason Merrill
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Pinski (QUIC) @ 2024-03-07 7:01 UTC (permalink / raw)
To: Andrew Pinski (QUIC), gcc-patches
> -----Original Message-----
> From: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> Sent: Tuesday, February 20, 2024 7:06 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Andrew Pinski (QUIC) <quic_apinski@quicinc.com>
> Subject: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript
> for qualified vector types [PR89224]
>
> After r7-987-gf17a223de829cb, the access for the elements of a vector type
> would lose the qualifiers.
> So if we had `constvector[0]`, the type of the element of the array would not
> have const on it.
> This was due to a missing build_qualified_type for the inner type of the vector
> when building the array type.
> We need to add back the call to build_qualified_type and now the access has
> the correct qualifiers. So the overloads and even if it is a lvalue or rvalue is
> correctly done.
>
> Note we correctly now reject the testcase gcc.dg/pr83415.c which was
> incorrectly accepted after r7-987-gf17a223de829cb.
Ping?
>
> Built and tested for aarch64-linux-gnu.
>
> PR c++/89224
>
> gcc/c-family/ChangeLog:
>
> * c-common.cc (convert_vector_to_array_for_subscript): Call
> build_qualified_type
> for the inner type.
>
> gcc/cp/ChangeLog:
>
> * constexpr.cc (cxx_eval_array_reference): Compare main variants
> for the vector/array types instead of the types directly.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/torture/vector-subaccess-1.C: New test.
> * gcc.dg/pr83415.c: Change warning to error.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
> gcc/c-family/c-common.cc | 7 +++++-
> gcc/cp/constexpr.cc | 3 ++-
> .../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
> gcc/testsuite/gcc.dg/pr83415.c | 2 +-
> 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644
> gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
>
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc index
> e15eff698df..884dd9043f9 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t
> loc,
> if (gnu_vector_type_p (TREE_TYPE (*vecp)))
> {
> tree type = TREE_TYPE (*vecp);
> + tree newitype;
>
> ret = !lvalue_p (*vecp);
>
> @@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript
> (location_t loc,
> for function parameters. */
> c_common_mark_addressable_vec (*vecp);
>
> + /* Make sure qualifiers are copied from the vector type to the new
> element
> + of the array type. */
> + newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS
> +(type));
> +
> *vecp = build1 (VIEW_CONVERT_EXPR,
> - build_array_type_nelts (TREE_TYPE (type),
> + build_array_type_nelts (newitype,
> TYPE_VECTOR_SUBPARTS (type)),
> *vecp);
> }
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index
> fa346fe01c9..1fe91d16e8e 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx
> *ctx, tree t,
> if (!lval
> && TREE_CODE (ary) == VIEW_CONVERT_EXPR
> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
> - && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
> + && TYPE_MAIN_VARIANT (TREE_TYPE (t))
> + == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND
> (ary,
> +0)))))
> ary = TREE_OPERAND (ary, 0);
>
> tree oldidx = TREE_OPERAND (t, 1);
> diff --git a/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
> b/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
> new file mode 100644
> index 00000000000..0c8958a4e03
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
> @@ -0,0 +1,23 @@
> +/* PR c++/89224 */
> +
> +/* The access of `vector[i]` has the same qualifiers as the original
> + vector which was missing. */
> +
> +typedef __attribute__((vector_size(16))) unsigned char Int8x8_t;
> +
> +template <class T>
> +void g(T &x) {
> + __builtin_abort();
> +}
> +template <class T>
> +void g(const T &x) {
> + __builtin_exit(0);
> +}
> +void f(const Int8x8_t x) {
> + g(x[0]);
> +}
> +int main(void)
> +{
> + Int8x8_t x ={};
> + f(x);
> +}
> diff --git a/gcc/testsuite/gcc.dg/pr83415.c b/gcc/testsuite/gcc.dg/pr83415.c
> index 5934c16d97c..2fc85031505 100644
> --- a/gcc/testsuite/gcc.dg/pr83415.c
> +++ b/gcc/testsuite/gcc.dg/pr83415.c
> @@ -7,6 +7,6 @@ int
> main (int argc, short *argv[])
> {
> int i = argc;
> - y[i] = 7 - i; /* { dg-warning "read-only" } */
> + y[i] = 7 - i; /* { dg-error "read-only" } */
> return 0;
> }
> --
> 2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
2024-02-21 3:06 [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224] Andrew Pinski
2024-03-07 7:01 ` Andrew Pinski (QUIC)
@ 2024-04-30 18:53 ` Jason Merrill
2024-04-30 19:04 ` Andrew Pinski
1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2024-04-30 18:53 UTC (permalink / raw)
To: Andrew Pinski, gcc-patches
On 2/20/24 19:06, Andrew Pinski wrote:
> After r7-987-gf17a223de829cb, the access for the elements of a vector type would lose the qualifiers.
> So if we had `constvector[0]`, the type of the element of the array would not have const on it.
> This was due to a missing build_qualified_type for the inner type of the vector when building the array type.
> We need to add back the call to build_qualified_type and now the access has the correct qualifiers. So the
> overloads and even if it is a lvalue or rvalue is correctly done.
>
> Note we correctly now reject the testcase gcc.dg/pr83415.c which was incorrectly accepted after r7-987-gf17a223de829cb.
>
> Built and tested for aarch64-linux-gnu.
>
> PR c++/89224
>
> gcc/c-family/ChangeLog:
>
> * c-common.cc (convert_vector_to_array_for_subscript): Call build_qualified_type
> for the inner type.
>
> gcc/cp/ChangeLog:
>
> * constexpr.cc (cxx_eval_array_reference): Compare main variants
> for the vector/array types instead of the types directly.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/torture/vector-subaccess-1.C: New test.
> * gcc.dg/pr83415.c: Change warning to error.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
> gcc/c-family/c-common.cc | 7 +++++-
> gcc/cp/constexpr.cc | 3 ++-
> .../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
> gcc/testsuite/gcc.dg/pr83415.c | 2 +-
> 4 files changed, 32 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
>
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> index e15eff698df..884dd9043f9 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
> if (gnu_vector_type_p (TREE_TYPE (*vecp)))
> {
> tree type = TREE_TYPE (*vecp);
> + tree newitype;
>
> ret = !lvalue_p (*vecp);
>
> @@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
> for function parameters. */
> c_common_mark_addressable_vec (*vecp);
>
> + /* Make sure qualifiers are copied from the vector type to the new element
> + of the array type. */
> + newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
> +
> *vecp = build1 (VIEW_CONVERT_EXPR,
> - build_array_type_nelts (TREE_TYPE (type),
> + build_array_type_nelts (newitype,
> TYPE_VECTOR_SUBPARTS (type)),
> *vecp);
> }
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index fa346fe01c9..1fe91d16e8e 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
> if (!lval
> && TREE_CODE (ary) == VIEW_CONVERT_EXPR
> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
> - && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
> + && TYPE_MAIN_VARIANT (TREE_TYPE (t))
> + == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))
Please add parens around the == expression so the formatting is stable.
With that change, OK for trunk and release branches.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
2024-04-30 18:53 ` Jason Merrill
@ 2024-04-30 19:04 ` Andrew Pinski
2024-04-30 19:12 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Pinski @ 2024-04-30 19:04 UTC (permalink / raw)
To: Jason Merrill; +Cc: Andrew Pinski, gcc-patches
On Tue, Apr 30, 2024 at 11:54 AM Jason Merrill <jason@redhat.com> wrote:
>
> On 2/20/24 19:06, Andrew Pinski wrote:
> > After r7-987-gf17a223de829cb, the access for the elements of a vector type would lose the qualifiers.
> > So if we had `constvector[0]`, the type of the element of the array would not have const on it.
> > This was due to a missing build_qualified_type for the inner type of the vector when building the array type.
> > We need to add back the call to build_qualified_type and now the access has the correct qualifiers. So the
> > overloads and even if it is a lvalue or rvalue is correctly done.
> >
> > Note we correctly now reject the testcase gcc.dg/pr83415.c which was incorrectly accepted after r7-987-gf17a223de829cb.
> >
> > Built and tested for aarch64-linux-gnu.
> >
> > PR c++/89224
> >
> > gcc/c-family/ChangeLog:
> >
> > * c-common.cc (convert_vector_to_array_for_subscript): Call build_qualified_type
> > for the inner type.
> >
> > gcc/cp/ChangeLog:
> >
> > * constexpr.cc (cxx_eval_array_reference): Compare main variants
> > for the vector/array types instead of the types directly.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/torture/vector-subaccess-1.C: New test.
> > * gcc.dg/pr83415.c: Change warning to error.
> >
> > Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> > ---
> > gcc/c-family/c-common.cc | 7 +++++-
> > gcc/cp/constexpr.cc | 3 ++-
> > .../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
> > gcc/testsuite/gcc.dg/pr83415.c | 2 +-
> > 4 files changed, 32 insertions(+), 3 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
> >
> > diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> > index e15eff698df..884dd9043f9 100644
> > --- a/gcc/c-family/c-common.cc
> > +++ b/gcc/c-family/c-common.cc
> > @@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
> > if (gnu_vector_type_p (TREE_TYPE (*vecp)))
> > {
> > tree type = TREE_TYPE (*vecp);
> > + tree newitype;
> >
> > ret = !lvalue_p (*vecp);
> >
> > @@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
> > for function parameters. */
> > c_common_mark_addressable_vec (*vecp);
> >
> > + /* Make sure qualifiers are copied from the vector type to the new element
> > + of the array type. */
> > + newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
> > +
> > *vecp = build1 (VIEW_CONVERT_EXPR,
> > - build_array_type_nelts (TREE_TYPE (type),
> > + build_array_type_nelts (newitype,
> > TYPE_VECTOR_SUBPARTS (type)),
> > *vecp);
> > }
> > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> > index fa346fe01c9..1fe91d16e8e 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
> > if (!lval
> > && TREE_CODE (ary) == VIEW_CONVERT_EXPR
> > && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
> > - && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
> > + && TYPE_MAIN_VARIANT (TREE_TYPE (t))
> > + == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))
>
> Please add parens around the == expression so the formatting is stable.
ok, I will make that change.
>
> With that change, OK for trunk and release branches.
For the GCC 14 branch, should I wait until after the release due to
RC1 going out today and I am not sure this counts as a show stopper
issue.
Thanks,
Andrew
>
> Jason
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
2024-04-30 19:04 ` Andrew Pinski
@ 2024-04-30 19:12 ` Jason Merrill
2024-05-02 6:50 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2024-04-30 19:12 UTC (permalink / raw)
To: Andrew Pinski; +Cc: Andrew Pinski, gcc-patches
On 4/30/24 12:04, Andrew Pinski wrote:
> On Tue, Apr 30, 2024 at 11:54 AM Jason Merrill <jason@redhat.com> wrote:
>>
>> On 2/20/24 19:06, Andrew Pinski wrote:
>>> After r7-987-gf17a223de829cb, the access for the elements of a vector type would lose the qualifiers.
>>> So if we had `constvector[0]`, the type of the element of the array would not have const on it.
>>> This was due to a missing build_qualified_type for the inner type of the vector when building the array type.
>>> We need to add back the call to build_qualified_type and now the access has the correct qualifiers. So the
>>> overloads and even if it is a lvalue or rvalue is correctly done.
>>>
>>> Note we correctly now reject the testcase gcc.dg/pr83415.c which was incorrectly accepted after r7-987-gf17a223de829cb.
>>>
>>> Built and tested for aarch64-linux-gnu.
>>>
>>> PR c++/89224
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>> * c-common.cc (convert_vector_to_array_for_subscript): Call build_qualified_type
>>> for the inner type.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> * constexpr.cc (cxx_eval_array_reference): Compare main variants
>>> for the vector/array types instead of the types directly.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * g++.dg/torture/vector-subaccess-1.C: New test.
>>> * gcc.dg/pr83415.c: Change warning to error.
>>>
>>> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
>>> ---
>>> gcc/c-family/c-common.cc | 7 +++++-
>>> gcc/cp/constexpr.cc | 3 ++-
>>> .../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
>>> gcc/testsuite/gcc.dg/pr83415.c | 2 +-
>>> 4 files changed, 32 insertions(+), 3 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
>>>
>>> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
>>> index e15eff698df..884dd9043f9 100644
>>> --- a/gcc/c-family/c-common.cc
>>> +++ b/gcc/c-family/c-common.cc
>>> @@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
>>> if (gnu_vector_type_p (TREE_TYPE (*vecp)))
>>> {
>>> tree type = TREE_TYPE (*vecp);
>>> + tree newitype;
>>>
>>> ret = !lvalue_p (*vecp);
>>>
>>> @@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
>>> for function parameters. */
>>> c_common_mark_addressable_vec (*vecp);
>>>
>>> + /* Make sure qualifiers are copied from the vector type to the new element
>>> + of the array type. */
>>> + newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
>>> +
>>> *vecp = build1 (VIEW_CONVERT_EXPR,
>>> - build_array_type_nelts (TREE_TYPE (type),
>>> + build_array_type_nelts (newitype,
>>> TYPE_VECTOR_SUBPARTS (type)),
>>> *vecp);
>>> }
>>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
>>> index fa346fe01c9..1fe91d16e8e 100644
>>> --- a/gcc/cp/constexpr.cc
>>> +++ b/gcc/cp/constexpr.cc
>>> @@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
>>> if (!lval
>>> && TREE_CODE (ary) == VIEW_CONVERT_EXPR
>>> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
>>> - && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
>>> + && TYPE_MAIN_VARIANT (TREE_TYPE (t))
>>> + == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))
>>
>> Please add parens around the == expression so the formatting is stable.
> ok, I will make that change.
>
>>
>> With that change, OK for trunk and release branches.
>
> For the GCC 14 branch, should I wait until after the release due to
> RC1 going out today and I am not sure this counts as a show stopper
> issue.
That's not my call ("all changes to the branch require a RM approval
now") but I think it can wait for 14.2.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
2024-04-30 19:12 ` Jason Merrill
@ 2024-05-02 6:50 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-05-02 6:50 UTC (permalink / raw)
To: Jason Merrill; +Cc: Andrew Pinski, Andrew Pinski, gcc-patches
On Tue, Apr 30, 2024 at 9:13 PM Jason Merrill <jason@redhat.com> wrote:
>
> On 4/30/24 12:04, Andrew Pinski wrote:
> > On Tue, Apr 30, 2024 at 11:54 AM Jason Merrill <jason@redhat.com> wrote:
> >>
> >> On 2/20/24 19:06, Andrew Pinski wrote:
> >>> After r7-987-gf17a223de829cb, the access for the elements of a vector type would lose the qualifiers.
> >>> So if we had `constvector[0]`, the type of the element of the array would not have const on it.
> >>> This was due to a missing build_qualified_type for the inner type of the vector when building the array type.
> >>> We need to add back the call to build_qualified_type and now the access has the correct qualifiers. So the
> >>> overloads and even if it is a lvalue or rvalue is correctly done.
> >>>
> >>> Note we correctly now reject the testcase gcc.dg/pr83415.c which was incorrectly accepted after r7-987-gf17a223de829cb.
> >>>
> >>> Built and tested for aarch64-linux-gnu.
> >>>
> >>> PR c++/89224
> >>>
> >>> gcc/c-family/ChangeLog:
> >>>
> >>> * c-common.cc (convert_vector_to_array_for_subscript): Call build_qualified_type
> >>> for the inner type.
> >>>
> >>> gcc/cp/ChangeLog:
> >>>
> >>> * constexpr.cc (cxx_eval_array_reference): Compare main variants
> >>> for the vector/array types instead of the types directly.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>> * g++.dg/torture/vector-subaccess-1.C: New test.
> >>> * gcc.dg/pr83415.c: Change warning to error.
> >>>
> >>> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> >>> ---
> >>> gcc/c-family/c-common.cc | 7 +++++-
> >>> gcc/cp/constexpr.cc | 3 ++-
> >>> .../g++.dg/torture/vector-subaccess-1.C | 23 +++++++++++++++++++
> >>> gcc/testsuite/gcc.dg/pr83415.c | 2 +-
> >>> 4 files changed, 32 insertions(+), 3 deletions(-)
> >>> create mode 100644 gcc/testsuite/g++.dg/torture/vector-subaccess-1.C
> >>>
> >>> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> >>> index e15eff698df..884dd9043f9 100644
> >>> --- a/gcc/c-family/c-common.cc
> >>> +++ b/gcc/c-family/c-common.cc
> >>> @@ -8936,6 +8936,7 @@ convert_vector_to_array_for_subscript (location_t loc,
> >>> if (gnu_vector_type_p (TREE_TYPE (*vecp)))
> >>> {
> >>> tree type = TREE_TYPE (*vecp);
> >>> + tree newitype;
> >>>
> >>> ret = !lvalue_p (*vecp);
> >>>
> >>> @@ -8950,8 +8951,12 @@ convert_vector_to_array_for_subscript (location_t loc,
> >>> for function parameters. */
> >>> c_common_mark_addressable_vec (*vecp);
> >>>
> >>> + /* Make sure qualifiers are copied from the vector type to the new element
> >>> + of the array type. */
> >>> + newitype = build_qualified_type (TREE_TYPE (type), TYPE_QUALS (type));
> >>> +
> >>> *vecp = build1 (VIEW_CONVERT_EXPR,
> >>> - build_array_type_nelts (TREE_TYPE (type),
> >>> + build_array_type_nelts (newitype,
> >>> TYPE_VECTOR_SUBPARTS (type)),
> >>> *vecp);
> >>> }
> >>> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> >>> index fa346fe01c9..1fe91d16e8e 100644
> >>> --- a/gcc/cp/constexpr.cc
> >>> +++ b/gcc/cp/constexpr.cc
> >>> @@ -4421,7 +4421,8 @@ cxx_eval_array_reference (const constexpr_ctx *ctx, tree t,
> >>> if (!lval
> >>> && TREE_CODE (ary) == VIEW_CONVERT_EXPR
> >>> && VECTOR_TYPE_P (TREE_TYPE (TREE_OPERAND (ary, 0)))
> >>> - && TREE_TYPE (t) == TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0))))
> >>> + && TYPE_MAIN_VARIANT (TREE_TYPE (t))
> >>> + == TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (TREE_OPERAND (ary, 0)))))
> >>
> >> Please add parens around the == expression so the formatting is stable.
> > ok, I will make that change.
> >
> >>
> >> With that change, OK for trunk and release branches.
> >
> > For the GCC 14 branch, should I wait until after the release due to
> > RC1 going out today and I am not sure this counts as a show stopper
> > issue.
>
> That's not my call ("all changes to the branch require a RM approval
> now") but I think it can wait for 14.2.
Yes, this should wait.
Richard.
> Jason
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-05-02 6:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-21 3:06 [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224] Andrew Pinski
2024-03-07 7:01 ` Andrew Pinski (QUIC)
2024-04-30 18:53 ` Jason Merrill
2024-04-30 19:04 ` Andrew Pinski
2024-04-30 19:12 ` Jason Merrill
2024-05-02 6:50 ` Richard Biener
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).