From: Andrew Pinski <pinskia@gmail.com>
To: Jason Merrill <jason@redhat.com>
Cc: Andrew Pinski <quic_apinski@quicinc.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++/c-common: Fix convert_vector_to_array_for_subscript for qualified vector types [PR89224]
Date: Tue, 30 Apr 2024 12:04:42 -0700 [thread overview]
Message-ID: <CA+=Sn1ndid69O=31s0v7Ur9r2Omf9NhxwVxunp5aqVEyoVMEtg@mail.gmail.com> (raw)
In-Reply-To: <fc2ac883-c53f-4566-af51-cad497fcf5ac@redhat.com>
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
>
next prev parent reply other threads:[~2024-04-30 19:04 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-21 3:06 Andrew Pinski
2024-03-07 7:01 ` Andrew Pinski (QUIC)
2024-04-30 18:53 ` Jason Merrill
2024-04-30 19:04 ` Andrew Pinski [this message]
2024-04-30 19:12 ` Jason Merrill
2024-05-02 6:50 ` Richard Biener
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+=Sn1ndid69O=31s0v7Ur9r2Omf9NhxwVxunp5aqVEyoVMEtg@mail.gmail.com' \
--to=pinskia@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=quic_apinski@quicinc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).