From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 48AE23855587 for ; Tue, 5 Sep 2023 14:56:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 48AE23855587 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 16A1111FB; Tue, 5 Sep 2023 07:57:16 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 59A6B3F64C; Tue, 5 Sep 2023 07:56:37 -0700 (PDT) From: Richard Sandiford To: Szabolcs Nagy Mail-Followup-To: Szabolcs Nagy ,, , , richard.sandiford@arm.com Cc: , , Subject: Re: [PATCH 06/11] aarch64: Fix pac-ret eh_return tests References: Date: Tue, 05 Sep 2023 15:56:36 +0100 In-Reply-To: (Szabolcs Nagy's message of "Tue, 22 Aug 2023 11:38:49 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-25.2 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP 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: Szabolcs Nagy writes: > This is needed since eh_return no longer prevents pac-ret in the > normal return path. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/return_address_sign_1.c: Move func4 to ... > * gcc.target/aarch64/return_address_sign_2.c: ... here and fix the > scan asm check. > * gcc.target/aarch64/return_address_sign_b_1.c: Move func4 to ... > * gcc.target/aarch64/return_address_sign_b_2.c: ... here and fix the > scan asm check. > --- > .../gcc.target/aarch64/return_address_sign_1.c | 13 +------------ > .../gcc.target/aarch64/return_address_sign_2.c | 17 +++++++++++++++-- > .../aarch64/return_address_sign_b_1.c | 11 ----------- > .../aarch64/return_address_sign_b_2.c | 17 +++++++++++++++-- > 4 files changed, 31 insertions(+), 27 deletions(-) > > diff --git a/gcc/testsuite/gcc.target/aarch64/return_address_sign_1.c b/gcc/testsuite/gcc.target/aarch64/return_address_sign_1.c > index 232ba67ade0..114a9dacb3f 100644 > --- a/gcc/testsuite/gcc.target/aarch64/return_address_sign_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/return_address_sign_1.c > @@ -37,16 +37,5 @@ func3 (int a, int b, int c) > /* autiasp */ > } > > -/* eh_return. */ > -void __attribute__ ((target ("arch=armv8.3-a"))) > -func4 (long offset, void *handler, int *ptr, int imm1, int imm2) > -{ > - /* no paciasp */ > - *ptr = imm1 + foo (imm1) + imm2; > - __builtin_eh_return (offset, handler); > - /* no autiasp */ > - return; > -} > - > -/* { dg-final { scan-assembler-times "autiasp" 3 } } */ > /* { dg-final { scan-assembler-times "paciasp" 3 } } */ > +/* { dg-final { scan-assembler-times "autiasp" 3 } } */ I suppose there is no normal return path here. I don't know how quickly we'd realise that though, in the sense that the flag register becomes known-1. But a quick-and-dirty check would be whether the exit block has a single predecessor, which in a function that calls eh_return should mean that the eh_return is unconditional. But that might not be worth worrying about, given the builtin's limited use case. And even if it is worth worrying about, keeping the test in this file would mix correctness with optimisation, which isn't a good thing for scan-assembler-times. So yeah, I agree this is OK. It should probably be part of 03 though, so that no individual commit causes a regression. Thanks, Richard > diff --git a/gcc/testsuite/gcc.target/aarch64/return_address_sign_2.c b/gcc/testsuite/gcc.target/aarch64/return_address_sign_2.c > index a4bc5b45333..d93492c3c43 100644 > --- a/gcc/testsuite/gcc.target/aarch64/return_address_sign_2.c > +++ b/gcc/testsuite/gcc.target/aarch64/return_address_sign_2.c > @@ -14,5 +14,18 @@ func1 (int a, int b, int c) > /* retaa */ > } > > -/* { dg-final { scan-assembler-times "paciasp" 1 } } */ > -/* { dg-final { scan-assembler-times "retaa" 1 } } */ > +/* eh_return. */ > +void __attribute__ ((target ("arch=armv8.3-a"))) > +func4 (long offset, void *handler, int *ptr, int imm1, int imm2) > +{ > + /* paciasp */ > + *ptr = imm1 + foo (imm1) + imm2; > + if (handler) > + /* br */ > + __builtin_eh_return (offset, handler); > + /* retaa */ > + return; > +} > + > +/* { dg-final { scan-assembler-times "paciasp" 2 } } */ > +/* { dg-final { scan-assembler-times "retaa" 2 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_1.c b/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_1.c > index 43e32ab6cb7..697fa30dc5a 100644 > --- a/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_1.c > @@ -37,16 +37,5 @@ func3 (int a, int b, int c) > /* autibsp */ > } > > -/* eh_return. */ > -void __attribute__ ((target ("arch=armv8.3-a"))) > -func4 (long offset, void *handler, int *ptr, int imm1, int imm2) > -{ > - /* no pacibsp */ > - *ptr = imm1 + foo (imm1) + imm2; > - __builtin_eh_return (offset, handler); > - /* no autibsp */ > - return; > -} > - > /* { dg-final { scan-assembler-times "pacibsp" 3 } } */ > /* { dg-final { scan-assembler-times "autibsp" 3 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_2.c b/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_2.c > index 9ed64ce0591..748924c72f3 100644 > --- a/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_2.c > +++ b/gcc/testsuite/gcc.target/aarch64/return_address_sign_b_2.c > @@ -14,5 +14,18 @@ func1 (int a, int b, int c) > /* retab */ > } > > -/* { dg-final { scan-assembler-times "pacibsp" 1 } } */ > -/* { dg-final { scan-assembler-times "retab" 1 } } */ > +/* eh_return. */ > +void __attribute__ ((target ("arch=armv8.3-a"))) > +func4 (long offset, void *handler, int *ptr, int imm1, int imm2) > +{ > + /* paciasp */ > + *ptr = imm1 + foo (imm1) + imm2; > + if (handler) > + /* br */ > + __builtin_eh_return (offset, handler); > + /* retab */ > + return; > +} > + > +/* { dg-final { scan-assembler-times "pacibsp" 2 } } */ > +/* { dg-final { scan-assembler-times "retab" 2 } } */