From: Kito Cheng <kito.cheng@sifive.com>
To: ebotcazou@adacore.com
Cc: GCC Patches <gcc-patches@gcc.gnu.org>,
Kito Cheng <kito.cheng@gmail.com>, Jim Wilson <jimw@sifive.com>,
Richard Biener <rguenther@suse.de>
Subject: Re: [PATCH v2] PR target/96759 - Handle global variable assignment from misaligned structure/PARALLEL return values.
Date: Tue, 13 Oct 2020 17:03:19 +0800 [thread overview]
Message-ID: <CALLt3TiUi9hOWqbeAHZBhX1KB_oKt4A4Pm3ijocX8fm9yPB-Vw@mail.gmail.com> (raw)
In-Reply-To: <CALLt3Tj49HqsgY-ALAGDLtEgqw6vr6vJ+EO9yizF5Msd=bOQ=w@mail.gmail.com>
ping^2
Hi Eric:
Do you mind having a review for that?
thanks :)
On Mon, Oct 5, 2020 at 5:24 PM Kito Cheng <kito.cheng@sifive.com> wrote:
> ping.
>
>
> On Fri, Sep 25, 2020 at 2:33 PM Richard Biener <rguenther@suse.de> wrote:
>
>> On Fri, 25 Sep 2020, Kito Cheng wrote:
>>
>> > In g:70cdb21e579191fe9f0f1d45e328908e59c0179e, DECL/global variable has
>> handled
>> > misaligned stores, but it didn't handle PARALLEL values, and I refer the
>> > other part of this function, I found the PARALLEL need handled by
>> > emit_group_* functions, so I add a check, and using emit_group_store if
>> > storing a PARALLEL value, also checked this change didn't break the
>> > testcase(gcc.target/arm/unaligned-argument-3.c) added by the orginal
>> changes.
>> >
>> > For riscv64 target, struct S {int a; double b;} will pack into a
>> parallel
>> > value to return and it has TImode when misaligned access is supported,
>> > however TImode required 16-byte align, but it only 8-byte align, so it
>> go to
>> > the misaligned stores handling, then it will try to generate move
>> > instruction from a PARALLEL value.
>> >
>> > Tested on following target without introduced new reguression:
>> > - riscv32/riscv64 elf
>> > - x86_64-linux
>> > - arm-eabi
>>
>> OK if Eric says so.
>>
>> Thanks,
>> Richard.
>>
>> > v2 changes:
>> > - Use maybe_emit_group_store instead of emit_group_store.
>> > - Remove push_temp_slots/pop_temp_slots, emit_group_store only require
>> > stack temp slot when dst is CONCAT or PARALLEL, however
>> > maybe_emit_group_store will always use REG for dst if needed.
>> >
>> > gcc/ChangeLog:
>> >
>> > PR target/96759
>> > * expr.c (expand_assignment): Handle misaligned stores with
>> PARALLEL
>> > value.
>> >
>> > gcc/testsuite/ChangeLog:
>> >
>> > PR target/96759
>> > * g++.target/riscv/pr96759.C: New.
>> > * gcc.target/riscv/pr96759.c: New.
>> > ---
>> > gcc/expr.c | 2 ++
>> > gcc/testsuite/g++.target/riscv/pr96759.C | 8 ++++++++
>> > gcc/testsuite/gcc.target/riscv/pr96759.c | 13 +++++++++++++
>> > 3 files changed, 23 insertions(+)
>> > create mode 100644 gcc/testsuite/g++.target/riscv/pr96759.C
>> > create mode 100644 gcc/testsuite/gcc.target/riscv/pr96759.c
>> >
>> > diff --git a/gcc/expr.c b/gcc/expr.c
>> > index 1a15f24b3979..6eb13a12c8c5 100644
>> > --- a/gcc/expr.c
>> > +++ b/gcc/expr.c
>> > @@ -5168,6 +5168,8 @@ expand_assignment (tree to, tree from, bool
>> nontemporal)
>> > rtx reg, mem;
>> >
>> > reg = expand_expr (from, NULL_RTX, VOIDmode, EXPAND_NORMAL);
>> > + /* Handle PARALLEL. */
>> > + reg = maybe_emit_group_store (reg, TREE_TYPE (from));
>> > reg = force_not_mem (reg);
>> > mem = expand_expr (to, NULL_RTX, VOIDmode, EXPAND_WRITE);
>> > if (TREE_CODE (to) == MEM_REF && REF_REVERSE_STORAGE_ORDER (to))
>> > diff --git a/gcc/testsuite/g++.target/riscv/pr96759.C
>> b/gcc/testsuite/g++.target/riscv/pr96759.C
>> > new file mode 100644
>> > index 000000000000..673999a4baf7
>> > --- /dev/null
>> > +++ b/gcc/testsuite/g++.target/riscv/pr96759.C
>> > @@ -0,0 +1,8 @@
>> > +/* { dg-options "-mno-strict-align -std=gnu++17" } */
>> > +/* { dg-do compile } */
>> > +struct S {
>> > + int a;
>> > + double b;
>> > +};
>> > +S GetNumbers();
>> > +auto [globalC, globalD] = GetNumbers();
>> > diff --git a/gcc/testsuite/gcc.target/riscv/pr96759.c
>> b/gcc/testsuite/gcc.target/riscv/pr96759.c
>> > new file mode 100644
>> > index 000000000000..621c39196fca
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.target/riscv/pr96759.c
>> > @@ -0,0 +1,13 @@
>> > +/* { dg-options "-mno-strict-align" } */
>> > +/* { dg-do compile } */
>> > +
>> > +struct S {
>> > + int a;
>> > + double b;
>> > +};
>> > +struct S GetNumbers();
>> > +struct S g;
>> > +
>> > +void foo(){
>> > + g = GetNumbers();
>> > +}
>> >
>>
>> --
>> Richard Biener <rguenther@suse.de>
>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>> Germany; GF: Felix Imend
>>
>
next prev parent reply other threads:[~2020-10-13 9:03 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-25 3:06 Kito Cheng
2020-09-25 6:33 ` Richard Biener
2020-10-05 9:24 ` Kito Cheng
2020-10-13 9:03 ` Kito Cheng [this message]
2020-10-13 9:38 ` Eric Botcazou
2020-10-14 3:56 ` Kito Cheng
2020-10-22 7:56 ` Kito Cheng
2020-10-22 7:58 ` Richard Biener
2020-10-22 8:38 ` Kito Cheng
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=CALLt3TiUi9hOWqbeAHZBhX1KB_oKt4A4Pm3ijocX8fm9yPB-Vw@mail.gmail.com \
--to=kito.cheng@sifive.com \
--cc=ebotcazou@adacore.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jimw@sifive.com \
--cc=kito.cheng@gmail.com \
--cc=rguenther@suse.de \
/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).