From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x635.google.com (mail-pl1-x635.google.com [IPv6:2607:f8b0:4864:20::635]) by sourceware.org (Postfix) with ESMTPS id 1D06E3858D33 for ; Tue, 6 Jun 2023 10:50:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D06E3858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pl1-x635.google.com with SMTP id d9443c01a7336-1b23f04e333so512505ad.3 for ; Tue, 06 Jun 2023 03:50:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1686048633; x=1688640633; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=forlcJ1SWUtUIbNIOiRgni5cPza4o85z8Gj8TTf138E=; b=lDMXato5PE6mWOA3JUhblTOvmQckR7pNZeuOC9NUY8If4IVdNt4MSs7Sq79Drhoucg tiKJ///Vmr8sPAoA3BPYr5L84nt2UYNGBzT7Q5Xu5+ibNKoFCodm5zXiEx4ygWJQt4rW W8MNKtnwW1YSBZa4OpWTKY6IOgMD4EhFGM9Pm1XdumWeS0gcIP21TxiXsiww1q+PsZ3s wz5+ZahQg+f/6qo3fILFk6xLzG+S5BAr4gMvZBvZwD2mXrDX07UnYuF67UROtWmz9VS3 eWDPL+k4R0cCJV3KVt3hLyfec3ukktEY1Kv7CkiGmPmqawyHJRmXPZOQhDAmdRWatEGj 0prg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686048633; x=1688640633; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=forlcJ1SWUtUIbNIOiRgni5cPza4o85z8Gj8TTf138E=; b=b7GeGnGnuveSVV2a28vMFqWJZb5aFMxXdI+qlx6zTt1MK2HeDO6DVKB5N22z21E5yG 5X5O14JwB5ft9d3NLxd3jgZzFi9asNoCMvGnZDE3ga6Vj70sNZCF/0CKjs4a8WM7Q2v6 T4/pEUbfro1mNP64WZpZ7EWQOktGnu20R3b564jTsNbV9GsPca2IkapSt/OffcWMtBGE Fo1fQrjsRnBihxlu+nucaY3W3lzSyYrSVEANLoRRrGiw4WUdWGnWNvWKP4xZg08R5TTr miCLxUDSOjDLa+2ldZ8jz0JZapn4EsoLZVgQI05DTY7xHJpgNJlLm9eLQsEOlLouG1Ao RKlQ== X-Gm-Message-State: AC+VfDwUj8PR8PJBGQ5CE7jgPfCpTCCux3L7D8aHSONbqligieMrsci7 rutNrVdtEpCSeYcWJfG9+H1jivWmp1gufeHIxHI= X-Google-Smtp-Source: ACHHUZ6bmVCNxezrWz3eUXj12XtFGAGuWI1EPqEvw0FQewnkR+dhYB2McWpYvZ+YcaUhZlYOTl5FuMyy6hXzV4WyGr0= X-Received: by 2002:a17:902:7b95:b0:1b2:2c0c:d3fa with SMTP id w21-20020a1709027b9500b001b22c0cd3famr636326pll.9.1686048632913; Tue, 06 Jun 2023 03:50:32 -0700 (PDT) MIME-Version: 1.0 References: <20230529103254.2753472-1-Jovan.Dmitrovic@Syrmia.com> In-Reply-To: From: YunQiang Su Date: Tue, 6 Jun 2023 18:50:21 +0800 Message-ID: Subject: Re: [PATCH] mips: Fix overaligned function arguments [PR109435] To: Jovan Dmitrovic Cc: "gcc-patches@gcc.gnu.org" , Djordje Todorovic Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,KAM_SHORT,RCVD_IN_DNSWL_NONE,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: Jovan Dmitrovic =E4=BA=8E2023=E5=B9=B46=E6=9C= =886=E6=97=A5=E5=91=A8=E4=BA=8C 18:29=E5=86=99=E9=81=93=EF=BC=9A > > I suppose that it is possible to check assembly. > Great. > Following is part of diff before and after my patch: > > 29,32c29,32 > < sd $6,0($2) > < sd $7,8($2) > < sd $8,16($2) > < sd $9,24($2) > --- > > sd $5,0($2) > > sd $6,8($2) > > sd $7,16($2) > > sd $8,24($2) > 63,66c63,66 > < sd $6,0($2) > < sd $7,8($2) > < sd $8,16($2) > < sd $9,24($2) > --- > > sd $5,0($2) > > sd $6,8($2) > > sd $7,16($2) > > sd $8,24($2) > 138,141c138,141 > < ld $6,64($16) > < ld $7,72($16) > < ld $8,80($16) > < ld $9,88($16) > --- > > ld $5,64($16) > > ld $6,72($16) > > ld $7,80($16) > > ld $8,88($16) > 148,151c148,151 > < ld $6,64($16) > < ld $7,72($16) > < ld $8,80($16) > < ld $9,88($16) > --- > > ld $5,64($16) > > ld $6,72($16) > > ld $7,80($16) > > ld $8,88($16) > 167,170c167,170 > < ld $6,0($16) > < ld $7,8($16) > < ld $8,16($16) > < ld $9,24($16) > --- > > ld $5,0($16) > > ld $6,8($16) > > ld $7,16($16) > > ld $8,24($16) > > What my patch effectively does it rearranges the data in > registers when invoking a function. I don't know whether > writing this testcase as an assembly check would make sense, > because that would make the testcase much less readable. I prefer an assembly check, because the test can be used even for cross building. It is not required, I guess. > ________________________________________ > From: YunQiang Su > Sent: Wednesday, May 31, 2023 12:05 PM > To: Jovan Dmitrovic > Cc: gcc-patches@gcc.gnu.org; Djordje Todorovic > Subject: Re: [PATCH] mips: Fix overaligned function arguments [PR109435] > > Jovan Dmitrovic =E4=BA=8E2023=E5=B9=B45=E6= =9C=8829=E6=97=A5=E5=91=A8=E4=B8=80 19:00=E5=86=99=E9=81=93=EF=BC=9A > > > > This patch changes alignment for typedef types when passed as > > arguments, making the alignment equal to the alignment of > > original (aliased) types. > > > > This change makes it impossible for a typedef type to have > > alignment that is less than its size. > > > > Signed-off-by: Jovan Dmitrovic > > > > gcc/ChangeLog: > > PR target/109435 > > * config/mips/mips.cc (mips_function_arg_alignment): Returns > > the alignment of function argument. In case of typedef type, > > it returns the aligment of the aliased type. > > (mips_function_arg_boundary): Relocated calculation of the > > aligment of function arguments. > > > > gcc/testsuite/ChangeLog: > > PR target/109435 > > * gcc.target/mips/align-1.c: New test. > > --- > > gcc/config/mips/mips.cc | 18 +++++++++++++- > > gcc/testsuite/gcc.target/mips/align-1.c | 33 +++++++++++++++++++++++++ > > 2 files changed, 50 insertions(+), 1 deletion(-) > > create mode 100644 gcc/testsuite/gcc.target/mips/align-1.c > > > > diff --git a/gcc/config/mips/mips.cc b/gcc/config/mips/mips.cc > > index ca822758b41..2019b7cd7d9 100644 > > --- a/gcc/config/mips/mips.cc > > +++ b/gcc/config/mips/mips.cc > > @@ -6190,6 +6190,22 @@ mips_arg_partial_bytes (cumulative_args_t cum, c= onst function_arg_info &arg) > > return info.stack_words > 0 ? info.reg_words * UNITS_PER_WORD : 0; > > } > > > > +/* Given MODE and TYPE of a function argument, return the alignment in > > + bits. In case of typedef, alignment of its original type is > > + used. */ > > + > > +static unsigned int > > +mips_function_arg_alignment (machine_mode mode, const_tree type) > > +{ > > + if (!type) > > + return GET_MODE_ALIGNMENT (mode); > > + > > + if (is_typedef_decl (TYPE_NAME (type))) > > + type =3D DECL_ORIGINAL_TYPE (TYPE_NAME (type)); > > + > > + return TYPE_ALIGN (type); > > +} > > + > > /* Implement TARGET_FUNCTION_ARG_BOUNDARY. Every parameter gets at > > least PARM_BOUNDARY bits of alignment, but will be given anything u= p > > to STACK_BOUNDARY bits if the type requires it. */ > > @@ -6198,8 +6214,8 @@ static unsigned int > > mips_function_arg_boundary (machine_mode mode, const_tree type) > > { > > unsigned int alignment; > > + alignment =3D mips_function_arg_alignment (mode, type); > > > > - alignment =3D type ? TYPE_ALIGN (type) : GET_MODE_ALIGNMENT (mode); > > if (alignment < PARM_BOUNDARY) > > alignment =3D PARM_BOUNDARY; > > if (alignment > STACK_BOUNDARY) > > diff --git a/gcc/testsuite/gcc.target/mips/align-1.c b/gcc/testsuite/gc= c.target/mips/align-1.c > > new file mode 100644 > > index 00000000000..816751b8099 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/mips/align-1.c > > @@ -0,0 +1,33 @@ > > +/* Check that typedef alignment does not affect passing of function > > + parameters. */ > > +/* { dg-do run { target { "mips*-*-linux*" } } } */ > > Is it possible to check the result with something like > scan-assembler > scan-assembler-not > instead of real running? > > > + > > +#include > > + > > +typedef struct ui8 > > +{ > > + unsigned v[8]; > > +} uint8 __attribute__ ((aligned(64))); > > + > > +unsigned > > +callee (int x, uint8 a) > > +{ > > + return a.v[0]; > > +} > > + > > +uint8 > > +identity (uint8 in) > > +{ > > + return in; > > +} > > + > > +int > > +main (void) > > +{ > > + uint8 vec =3D {{1, 2, 3, 4, 5, 6, 7, 8}}; > > + uint8 temp =3D identity (vec); > > + unsigned temp2 =3D callee (1, identity (vec)); > > + assert (callee (1, temp) =3D=3D 1); > > + assert (temp2 =3D=3D 1); > > + return 0; > > +} > > -- > > 2.34.1 > > --=20 YunQiang Su