From patchwork Mon Oct 17 12:41:24 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wilco Dijkstra X-Patchwork-Id: 682899 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3syHs42yjNz9sDG for ; Mon, 17 Oct 2016 23:42:00 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=iga4hXwd; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :mime-version:content-type:content-transfer-encoding; q=dns; s= default; b=tYP2gKLPdmipqUlqicx5jsOGEoS+hJKUP3iT9Aa8MxCISo4uRpieR RVkce/8TcZHMyWsvPlajruPM3FgPEwpuCwaD238R0KGrOdZuVtvjBPIZPqs9Iss9 B+9EFtga1T4E6bVbb129hrLkzjyIfNilG9Zcz4WcNG0VzBN3jvLqDk= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:references:in-reply-to :mime-version:content-type:content-transfer-encoding; s=default; bh=ojij8cy70oc2Wcz/VA0iGkK/cHE=; b=iga4hXwdA32L/4hHOgw85vMo+q9C dEa4AE/ukcUN71aIaCWh1KuEqkndqQXKLiNW4BczDQm6XJ6blbv5QXdUozllrYH7 2c82hMuDC61Bt7kt3lrk1RX95oBsaDwwfhMyhzbRT9YgqySHt3haZE/h3afxm9Go drgfd4p9yJo5gjI= Received: (qmail 79287 invoked by alias); 17 Oct 2016 12:41:40 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 79201 invoked by uid 89); 17 Oct 2016 12:41:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, KAM_LOTSOFHASH, SPF_PASS autolearn=no version=3.3.2 spammy=aint, dawn, DSE, detect 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; Mon, 17 Oct 2016 12:41:29 +0000 Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01lp0208.outbound.protection.outlook.com [213.199.154.208]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-8-i19B5aWJNS60HGQVY4GggQ-1; Mon, 17 Oct 2016 13:41:25 +0100 Received: from AM5PR0802MB2610.eurprd08.prod.outlook.com (10.175.46.18) by AM5PR0802MB2612.eurprd08.prod.outlook.com (10.175.46.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.659.11; Mon, 17 Oct 2016 12:41:24 +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.0659.025; Mon, 17 Oct 2016 12:41:24 +0000 From: Wilco Dijkstra To: Ramana Radhakrishnan , GCC Patches CC: nd Subject: Re: [PATCH][AArch64 - v3] Simplify eh_return implementation Date: Mon, 17 Oct 2016 12:41:24 +0000 Message-ID: References: , , In-Reply-To: x-ms-office365-filtering-correlation-id: ffbac26a-f070-4bce-4eb1-08d3f68ae7a2 x-microsoft-exchange-diagnostics: 1; AM5PR0802MB2612; 7:B9nH3+mSjEslIp8j+s/cTBHxieNA7KNADqxf3V9U4xw5zNgnwHPa5T+ssVbjqhqGswQeENzTyKDzuLwrf04djQE3N1XQxsqmlVPWwVzUP1XuGHRxOqTViWOOQpiNEZAMhc3FXlFOmBUkv7ETPCooOru+mXQg1KvSVBnSZWkprCkUR39n38TJyKNaqLIBb+oNFOIpXlP8W//+OSkxB9wp44xUieSN/IiCszwmYnmiC6OBULYh3HHSB22BtZWDsdd5OOdDLSRrxX1LK/lvRhJ44T02zWuaQGvrPZM5qNFEKp/82FDT63Y1UC86rUh4FOa8XChYQiVAtj7Qs1x5QaDiFP2ALwausJWImLIJkGMua/E= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2612; 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)(8121501046)(5005006)(3002001)(10201501046)(6055026); SRVR:AM5PR0802MB2612; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0802MB2612; x-forefront-prvs: 0098BA6C6C x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(199003)(24454002)(54534003)(189002)(377424004)(50986999)(5660300001)(33656002)(8936002)(2950100002)(101416001)(76176999)(54356999)(102836003)(586003)(8676002)(7846002)(4326007)(575784001)(122556002)(86362001)(76576001)(450100001)(3660700001)(305945005)(189998001)(2906002)(19580395003)(77096005)(19580405001)(3900700001)(5002640100001)(7696004)(2900100001)(93886004)(11100500001)(106116001)(10400500002)(87936001)(81166006)(6116002)(105586002)(68736007)(9686002)(3846002)(97736004)(81156014)(7736002)(3280700002)(66066001)(5001770100001)(92566002)(74316002)(106356001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0802MB2612; 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: 17 Oct 2016 12:41:24.0604 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2612 X-MC-Unique: i19B5aWJNS60HGQVY4GggQ-1 ping From: Wilco Dijkstra Sent: 02 September 2016 12:31 To: Ramana Radhakrishnan; GCC Patches Cc: nd Subject: Re: [PATCH][AArch64 - v3] Simplify eh_return implementation   Ramana Radhakrishnan wrote: > Can you please file a PR for this and add some testcases ?  This sounds like a serious enough problem that needs to be looked at probably going back 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 use of the frame pointer so the return location is always at FP + 8.  This means we can 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 backport 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/aarch64-protos.h index 3cdd69b8af1089a839e5d45cda94bc70a15cd777..327c0a97f6f687604afef249b79ac22628418070 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..fa81e4b853dafcccc08842955288861ec7e7acca 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)   -/* 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_REGNUM) +/* 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 ()    /* 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..f07d771ea343803e054e03f59c8c1efb698bf474 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;   +  /* Force a frame pointer for EH returns so the return address is at FP+8.  */ +  if (crtl->calls_eh_return) +    return true; +    return false;  }   @@ -3298,7 +3302,8 @@ aarch64_expand_epilogue (bool for_sibcall)                           + cfun->machine->frame.saved_varargs_size) != 0;      /* 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 = false; @@ -3366,52 +3371,15 @@ aarch64_expand_epilogue (bool for_sibcall)      emit_jump_insn (ret_rtx);  }   -/* 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 = 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 = FP and hence -     we return a SP-relative location since all the addresses are SP-relative -     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 = gen_frame_mem (Pmode, +    plus_constant (Pmode, hard_frame_pointer_rtx, UNITS_PER_WORD)); +  MEM_VOLATILE_P (tmp) = true; +  return tmp;  }    /* 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..7d86aa7c0cb2fdb30889badc172d2270eeadb1e5 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -591,25 +591,6 @@    [(set_attr "type" "branch")]  )   -(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] = 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/gcc.target/aarch64/eh_return.c new file mode 100644 index 0000000000000000000000000000000000000000..32179488085ed4c84aa77007565c875d09c4197c --- /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 = (int)(long)p & 7; +  if (val) +    abort (); +} + +void +eh1 (void *p, int x) +{ +  void *q = __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 = 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 = 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 == 0) +    do_test1 (); +  if (test == 1) +    do_test2 (); +  if (failed || test != 2) +    exit (1); +  exit (0); +}