From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 2240B3857C7B for ; Fri, 25 Sep 2020 06:33:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2240B3857C7B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 1E20EAE14; Fri, 25 Sep 2020 06:33:33 +0000 (UTC) Date: Fri, 25 Sep 2020 08:33:32 +0200 (CEST) From: Richard Biener Sender: rguenther@c653.arch.suse.de To: Kito Cheng cc: gcc-patches@gcc.gnu.org, kito.cheng@gmail.com, jimw@sifive.com, ebotcazou@adacore.com Subject: Re: [PATCH v2] PR target/96759 - Handle global variable assignment from misaligned structure/PARALLEL return values. In-Reply-To: <20200925030651.67653-1-kito.cheng@sifive.com> Message-ID: References: <20200925030651.67653-1-kito.cheng@sifive.com> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Sep 2020 06:33:35 -0000 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 SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imend