From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id 16F6A3858CDB for ; Wed, 24 May 2023 10:07:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 16F6A3858CDB Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn Received: from loongson.cn (unknown [10.20.4.52]) by gateway (Coremail) with SMTP id _____8Dx+fHm4W1kOGMAAA--.1143S3; Wed, 24 May 2023 18:07:34 +0800 (CST) Received: from [10.20.4.52] (unknown [10.20.4.52]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxZbXj4W1ktcJzAA--.61235S2; Wed, 24 May 2023 18:07:32 +0800 (CST) Subject: Re: [PATCH] LoongArch: Fix the problem of structure parameter passing in C++. This structure has empty structure members and less than three floating point members. To: Xi Ruoyao , gcc-patches@gcc.gnu.org Cc: i@xen0n.name, xuchenghua@loongson.cn References: <20230524060407.19181-1-chenglulu@loongson.cn> <2d33bf204b0d59f16df8714123ee812be5754617.camel@xry111.site> <50e25f56-7b00-7b5b-60fe-3b9dac60bf27@loongson.cn> <40e9c859a7e71a04976b776c93d5c513cb604fcb.camel@xry111.site> From: Lulu Cheng Message-ID: Date: Wed, 24 May 2023 18:07:31 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <40e9c859a7e71a04976b776c93d5c513cb604fcb.camel@xry111.site> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID:AQAAf8CxZbXj4W1ktcJzAA--.61235S2 X-CM-SenderInfo: xfkh0wpoxo3qxorr0wxvrqhubq/ X-Coremail-Antispam: 1Uk129KBjvJXoWxZw1UGFykXFWruF4UWF18Grg_yoWrZF17pF WFka48Gan7Jr1fur129w4kZFWSv3yxJr9rAryrJ34vvF98KFyrJF40ya15uF9xCws5WrsF vw4YgrW7uryDZaDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU baxYFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_JrI_Jryl8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVW8JVW5JwA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8JVWxJwA2z4 x0Y4vEx4A2jsIE14v26r4UJVWxJr1l84ACjcxK6I8E87Iv6xkF7I0E14v26F4UJVW0owAS 0I0E0xvYzxvE52x082IY62kv0487Mc804VCY07AIYIkI8VC2zVCFFI0UMc02F40EFcxC0V AKzVAqx4xG6I80ewAv7VC0I7IYx2IY67AKxVWUJVWUGwAv7VC2z280aVAFwI0_Jr0_Gr1l Ox8S6xCaFVCjc4AY6r1j6r4UM4x0Y48IcVAKI48JMxk0xIA0c2IEe2xFo4CEbIxvr21l42 xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1l4IxYO2xFxVAFwI0_Jrv_JF1l x2IqxVAqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14 v26r126r1DMIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IY x2IY6xkF7I0E14v26r1j6r4UMIIF0xvE42xK8VAvwI8IcIk0rVWUJVWUCwCI42IY6I8E87 Iv67AKxVWUJVW8JwCI42IY6I8E87Iv6xkF7I0E14v26r1j6r4UYxBIdaVFxhVjvjDU0xZF pf9x07j8yCJUUUUU= X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00,KAM_DMARC_STATUS,KAM_SHORT,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 在 2023/5/24 下午5:25, Xi Ruoyao 写道: > On Wed, 2023-05-24 at 16:47 +0800, Lulu Cheng wrote: >> 在 2023/5/24 下午2:45, Xi Ruoyao 写道: >>> On Wed, 2023-05-24 at 14:04 +0800, Lulu Cheng wrote: >>>> An empty struct type that is not non-trivial for the purposes of >>>> calls >>>> will be treated as though it were the following C type: >>>> >>>> struct { >>>>    char c; >>>> }; >>>> >>>> Before this patch was added, a structure parameter containing an >>>> empty structure and >>>> less than three floating-point members was passed through one or >>>> two floating-point >>>> registers, while nested empty structures are ignored. Which did >>>> not conform to the >>>> calling convention. >>> No, it's a deliberate decision I've made in >>> https://gcc.gnu.org/r12-8294.  And we already agreed "the ABI needs >>> to >>> be updated" when we applied r12-8294, but I've never improved my >>> English >>> skill to revise the ABI myself :(. >>> >>> We are also using the same "de-facto" ABI throwing away the empty >>> struct >>> for Clang++ (https://reviews.llvm.org/D132285).  So we should update >>> the >>> spec here, instead of changing every implementation. >>> >>> The C++ standard treats the empty struct as size 1 for ensuring the >>> semantics of pointer comparison operations.  When we pass it through >>> the >>> registers, there is no need to really consider the empty field >>> because >>> there is no pointers to registers. >>> >> I think that the rules for passing parameters to empty structures or >> nested empty structures should be unified, > There is no need to unify them because "passing a struct" is already > different from "passing its members one by one". Say: > > int f1(int a, int b); > > and > > int f2(struct {int a, b;} ab); > > "a" and "b" are already passed differently. I mean I think that empty structs in st1 and st2 should be treated the same way in the way of passing parameters. > >> but the current implementation in gcc is as follows(in C++): >> >> Compare the two structures,the current implementation is as follows: >> >> struct st1 >> { >>    struct empty {} e1; >>    long a; >>    long b; >> }; >> >> passed by reference. >> >> >> struct st2 >> { >>    struct empty {} e1; >>    double f0; >>    double f1; >> }; >> >> passed through two floating-point registers. > Well this is nasty, but it is the same behavior as RISC-V: > https://godbolt.org/z/fEexq148r > > I deliberately made our logic similar to RISC-V in r12-8294 because > "there seems no reason to do it differently". Maybe I was wrong and we > should have ignored st1::e1 as well (but IIRC we were running out of > time for GCC 12 release so we didn't have time to consider this :( ). > > But now it's better to "keep the current behavior as-is" because: > > 1. The current behavior of GCC and Clang already matches and the > behavior is kept since the day one GCC and Clang supports LoongArch. So > there is currently no ABI incompatibility in practice, but changing the > behavior will introduce an ABI incompatibility. The parameter passing rules for a single empty structure are different in GCC and Clang. eg: void test (struct empty, int a); In GCC, the empty structure is passed through $a0, and the variable a is passed through $a1, but Clang passes a through $a0, and the empty structure is ignored. > 2. Changing the behavior will make the compiler more complex, and > slower. > 3. Changing the behavior will need a -Wpsabi warning according to the > GCC policy, leading to more boring code (and more slow-down) in the > compiler. I really understand and thank you for your concerns, we have also considered the issue of compatibility. Before the modification, we made an assessment. The colleagues of the operating system built a total of 3,300 linux basic packages, and only one package was affected by this modification. This is why GCC fixes this as a bug without adding -Wpsabi. > >> Judging from the size of the structure, the size of st2 is already >> larger than 2xGRLEN, should be passed by reference. > I'd consider it a flaw in the psABI doc because it was obviously written > w/o considering the possibility of a zero-sized field in an aggregate. > > I knew this flaw when I created r12-8294 and I planned to revise the > psABI doc for it, but I've never improved my English skill enough to do > the work. I'm considering copying some word from RISC-V psABI if there > is no copyright issue. >