From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122715 invoked by alias); 2 Sep 2016 11:31:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 122706 invoked by uid 89); 2 Sep 2016 11:31:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.5 required=5.0 tests=AWL,BAYES_60,KAM_LOTSOFHASH,SPF_PASS autolearn=no version=3.3.2 spammy=Pmode, pmode, cselib, if_then_else X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 02 Sep 2016 11:31:09 +0000 Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01lp0239.outbound.protection.outlook.com [213.199.154.239]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-84-xcD-tUKOOB2AHXii5QgLQA-1; Fri, 02 Sep 2016 12:31:05 +0100 Received: from AM5PR0802MB2610.eurprd08.prod.outlook.com (10.175.46.18) by AM5PR0802MB2610.eurprd08.prod.outlook.com (10.175.46.18) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.599.9; Fri, 2 Sep 2016 11:31:04 +0000 Received: from AM5PR0802MB2610.eurprd08.prod.outlook.com ([10.175.46.18]) by AM5PR0802MB2610.eurprd08.prod.outlook.com ([10.175.46.18]) with mapi id 15.01.0599.010; Fri, 2 Sep 2016 11:31:04 +0000 From: Wilco Dijkstra To: Ramana Radhakrishnan , GCC Patches CC: nd Subject: Re: [PATCH][AArch64 - v3] Simplify eh_return implementation Date: Fri, 02 Sep 2016 11:31:00 -0000 Message-ID: References: , In-Reply-To: x-ms-office365-filtering-correlation-id: c04dcd70-d58d-4ea2-0dd5-08d3d324a007 x-microsoft-exchange-diagnostics: 1;AM5PR0802MB2610;20:Gq1j4vMRNbgd7F5MRFd17dyE/psqbq9CkpWyPwA1L6wNkFJFVMbCBy3TXchOM+uberqzwSeQ90eiDmzR3z9i74Km+Yisi/zeAflK7OtOi9iTQDGPEnfEvIHoNDsX28Za82tbOvmsJwz6PldRKOAA8qxB+jM5etu0mSbQa6H1WRM= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2610; nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026);SRVR:AM5PR0802MB2610;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2610; x-forefront-prvs: 00531FAC2C x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(377424004)(199003)(189002)(54534003)(24454002)(4326007)(81156014)(3660700001)(106356001)(5660300001)(66066001)(106116001)(105586002)(19580405001)(7846002)(575784001)(76576001)(19580395003)(450100001)(6116002)(102836003)(5001770100001)(189998001)(81166006)(7696003)(2906002)(87936001)(68736007)(2900100001)(3846002)(11100500001)(92566002)(5002640100001)(33656002)(3280700002)(9686002)(7736002)(586003)(305945005)(74316002)(2950100001)(76176999)(50986999)(86362001)(54356999)(8676002)(10400500002)(77096005)(97736004)(122556002)(8936002)(101416001);DIR:OUT;SFP:1101;SCL:1;SRVR:AM5PR0802MB2610;H:AM5PR0802MB2610.eurprd08.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Sep 2016 11:31:04.4640 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2610 X-MC-Unique: xcD-tUKOOB2AHXii5QgLQA-1 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2016-09/txt/msg00077.txt.bz2 Ramana Radhakrishnan wrote: > Can you please file a PR for this and add some testcases ?=A0 This sounds= like a serious enough problem that needs to be looked at probably going ba= ck since the dawn of time. I've created PR77455. Updated patch below: This patch simplifies the handling of the EH return value. We force the us= e of the frame pointer so the return location is always at FP + 8. This means we ca= n emit a simple volatile access in EH_RETURN_HANDLER_RTX without needing md patterns, splitters and frame offset calculations. The new implementation = also fixes various bugs in aarch64_final_eh_return_addr, which does not work with -fomit-frame-pointer, alloca or outgoing arguments. Bootstrap OK, GCC Regression OK, OK for trunk? Would it be useful to backpo= rt this to GCC6.x? ChangeLog: 2016-09-02 Wilco Dijkstra PR77455 gcc/ * config/aarch64/aarch64.md (eh_return): Remove pattern and splitter. * config/aarch64/aarch64.h (AARCH64_EH_STACKADJ_REGNUM): Remove. (EH_RETURN_HANDLER_RTX): New define. * config/aarch64/aarch64.c (aarch64_frame_pointer_required): Force frame pointer in EH return functions. (aarch64_expand_epilogue): Add barrier for eh_return. (aarch64_final_eh_return_addr): Remove. (aarch64_eh_return_handler_rtx): New function. * config/aarch64/aarch64-protos.h (aarch64_final_eh_return_addr): Remove. (aarch64_eh_return_handler_rtx): New prototype. testsuite/ * gcc.target/aarch64/eh_return.c: New test. -- diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch= 64-protos.h index 3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79= ac22628418070 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -358,7 +358,7 @@ int aarch64_hard_regno_mode_ok (unsigned, machine_mode); int aarch64_hard_regno_nregs (unsigned, machine_mode); int aarch64_uxt_size (int, HOST_WIDE_INT); int aarch64_vec_fpconst_pow_of_2 (rtx); -rtx aarch64_final_eh_return_addr (void); +rtx aarch64_eh_return_handler_rtx (void); rtx aarch64_mask_from_zextract_ops (rtx, rtx); const char *aarch64_output_move_struct (rtx *operands); rtx aarch64_return_addr (int, rtx); diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 003fec87e41db618570663f28cc2387a87e8252a..fa81e4b853dafcccc0884295528= 8861ec7e7acca 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -400,9 +400,9 @@ extern unsigned aarch64_architecture_version; #define ASM_DECLARE_FUNCTION_NAME(STR, NAME, DECL) \ aarch64_declare_function_name (STR, NAME, DECL) =20 -/* The register that holds the return address in exception handlers. */ -#define AARCH64_EH_STACKADJ_REGNUM (R0_REGNUM + 4) -#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, AARCH64_EH_STACKADJ_REG= NUM) +/* For EH returns X4 contains the stack adjustment. */ +#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, R4_REGNUM) +#define EH_RETURN_HANDLER_RTX aarch64_eh_return_handler_rtx () =20 /* Don't use __builtin_setjmp until we've defined it. */ #undef DONT_USE_BUILTIN_SETJMP diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index e742c19d76e6c62117aa62a990b9c2945aa06b74..f07d771ea343803e054e03f59c8= c1efb698bf474 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -2739,6 +2739,10 @@ aarch64_frame_pointer_required (void) && (!crtl->is_leaf || df_regs_ever_live_p (LR_REGNUM))) return true; =20 + /* Force a frame pointer for EH returns so the return address is at FP+8= . */ + if (crtl->calls_eh_return) + return true; + return false; } =20 @@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall) + cfun->machine->frame.saved_varargs_size) !=3D 0; =20 /* Emit a barrier to prevent loads from a deallocated stack. */ - if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca) + if (final_adjust > crtl->outgoing_args_size || cfun->calls_alloca + || crtl->calls_eh_return) { emit_insn (gen_stack_tie (stack_pointer_rtx, stack_pointer_rtx)); need_barrier_p =3D false; @@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall) emit_jump_insn (ret_rtx); } =20 -/* Return the place to copy the exception unwinding return address to. - This will probably be a stack slot, but could (in theory be the - return register). */ +/* Implement EH_RETURN_HANDLER_RTX. The return address is stored at FP + = 8. + The access needs to be volatile to prevent it from being removed. */ rtx -aarch64_final_eh_return_addr (void) +aarch64_eh_return_handler_rtx (void) { - HOST_WIDE_INT fp_offset; - - aarch64_layout_frame (); - - fp_offset =3D cfun->machine->frame.frame_size - - cfun->machine->frame.hard_fp_offset; - - if (cfun->machine->frame.reg_offset[LR_REGNUM] < 0) - return gen_rtx_REG (DImode, LR_REGNUM); - - /* DSE and CSELIB do not detect an alias between sp+k1 and fp+k2. This = can - result in a store to save LR introduced by builtin_eh_return () being - incorrectly deleted because the alias is not detected. - So in the calculation of the address to copy the exception unwinding - return address to, we note 2 cases. - If FP is needed and the fp_offset is 0, it means that SP =3D FP and h= ence - we return a SP-relative location since all the addresses are SP-relat= ive - in this case. This prevents the store from being optimized away. - If the fp_offset is not 0, then the addresses will be FP-relative and - therefore we return a FP-relative location. */ - - if (frame_pointer_needed) - { - if (fp_offset) - return gen_frame_mem (DImode, - plus_constant (Pmode, hard_frame_pointer_rtx, UNITS_PER_WORD)); - else - return gen_frame_mem (DImode, - plus_constant (Pmode, stack_pointer_rtx, UNITS_PER_WORD)); - } - - /* If FP is not needed, we calculate the location of LR, which would be - at the top of the saved registers block. */ - - return gen_frame_mem (DImode, - plus_constant (Pmode, - stack_pointer_rtx, - fp_offset - + cfun->machine->frame.saved_regs_size - - 2 * UNITS_PER_WORD)); + rtx tmp =3D gen_frame_mem (Pmode, + plus_constant (Pmode, hard_frame_pointer_rtx, UNITS_PER_WORD)); + MEM_VOLATILE_P (tmp) =3D true; + return tmp; } =20 /* Output code to add DELTA to the first argument, and then jump diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 21f5a6aba74d28f04b9391ba917453a4cd7de1af..7d86aa7c0cb2fdb30889badc172= d2270eeadb1e5 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -591,25 +591,6 @@ [(set_attr "type" "branch")] ) =20 -(define_insn "eh_return" - [(unspec_volatile [(match_operand:DI 0 "register_operand" "r")] - UNSPECV_EH_RETURN)] - "" - "#" - [(set_attr "type" "branch")] - -) - -(define_split - [(unspec_volatile [(match_operand:DI 0 "register_operand" "")] - UNSPECV_EH_RETURN)] - "reload_completed" - [(set (match_dup 1) (match_dup 0))] - { - operands[1] =3D aarch64_final_eh_return_addr (); - } -) - (define_insn "*cb1" [(set (pc) (if_then_else (EQL (match_operand:GPI 0 "register_operand" "r= ") (const_int 0)) diff --git a/gcc/testsuite/gcc.target/aarch64/eh_return.c b/gcc/testsuite/g= cc.target/aarch64/eh_return.c new file mode 100644 index 0000000000000000000000000000000000000000..32179488085ed4c84aa77007565= c875d09c4197c --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/eh_return.c @@ -0,0 +1,82 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-inline" } */ + +#include +#include + +int val, test, failed; + +int main (void); + +void +eh0 (void *p) +{ + val =3D (int)(long)p & 7; + if (val) + abort (); +} + +void +eh1 (void *p, int x) +{ + void *q =3D __builtin_alloca (x); + eh0 (q); + __builtin_eh_return (0, p); +} + +void +eh2a (int a,int b,int c,int d,int e,int f,int g,int h, void *p) +{ + val =3D a + b + c + d + e + f + g + h + (int)(long)p & 7; +} + +void +eh2 (void *p) +{ + eh2a (val, val, val, val, val, val, val, val, p); + __builtin_eh_return (0, p); +} + + +void +continuation (void) +{ + test++; + main (); +} + +void +fail (void) +{ + failed =3D 1; + printf ("failed\n"); + continuation (); +} + +void +do_test1 (void) +{ + if (!val) + eh1 (continuation, 100); + fail (); +} + +void +do_test2 (void) +{ + if (!val) + eh2 (continuation); + fail (); +} + +int +main (void) +{ + if (test =3D=3D 0) + do_test1 (); + if (test =3D=3D 1) + do_test2 (); + if (failed || test !=3D 2) + exit (1); + exit (0); +}