From patchwork Thu Sep 1 12:33:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Wilco Dijkstra X-Patchwork-Id: 664913 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 3sQ1rl3X2Sz9s2k for ; Thu, 1 Sep 2016 22:33:41 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=o7fVB9cF; 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=gO5yiIFwI4r0NRuyaDE4BFaNhts5tFN+Dde8Dbaub4PnaXIAHra+x 9LiK6PJiRghAd9vy36RXBv6sehZcKy6EB7dedE3oMxETS/R+uvnSZDuPrK7UKnLk KBG/942e1utdinKY+w4O/SXjY3CBptVfr+ET2eV16h0F1gw613ZQWM= 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=rTMXBIx466lwLPjD57oCDmbD8oI=; b=o7fVB9cFplIy34QCtTwC8Yu+g8XB lh+exKfUi8iZdcs4PuBJtLK8wSQsrlpJe1E4OzIPZoQNPBt7ES6GA4NAEWAAV4O0 99WL40zR5xwQiRF/yrBN0YbOZzR06w/A6TN7tEj2bF6PgXBb8CUjukZfxcaDlEg4 sC5cAsWT+rlTQxU= Received: (qmail 57554 invoked by alias); 1 Sep 2016 12:33:33 -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 57544 invoked by uid 89); 1 Sep 2016 12:33:33 -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_00, KAM_LOTSOFHASH, SPF_PASS autolearn=no version=3.3.2 spammy=machine_mode, Pmode, pmode, cselib X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Sep 2016 12:33:22 +0000 Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01lp0242.outbound.protection.outlook.com [213.199.154.242]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-85-2ly9MVcVOm6hjrAI8rDdjQ-1; Thu, 01 Sep 2016 13:33:19 +0100 Received: from AM5PR0802MB2610.eurprd08.prod.outlook.com (10.175.46.18) by AM5PR0802MB2611.eurprd08.prod.outlook.com (10.175.46.19) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA_P384) id 15.1.599.9; Thu, 1 Sep 2016 12:33:18 +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; Thu, 1 Sep 2016 12:33:18 +0000 From: Wilco Dijkstra To: GCC Patches CC: nd Subject: Re: [PATCH][AArch64 - v2] Simplify eh_return implementation Date: Thu, 1 Sep 2016 12:33:17 +0000 Message-ID: References: , , In-Reply-To: x-ms-office365-filtering-correlation-id: f961b909-3275-4826-5cd7-08d3d26426f3 x-microsoft-exchange-diagnostics: 1; AM5PR0802MB2611; 20:+e3wUIvqRH/aYayOVYNotd1lheOAcCUwQchzzn/2g3Ppl9cPlya25UJ/2jAqLKEnsBtn+iYW5v6mq1WSRqu7s+7Zs/LlIUOc2V6QqOvEyJhTGRdIURCwvCx4XtBQGgPMsQMsjpdjd7tbQlzPsbFn9xF5LTyvzIbiWkWJEfvoiHg= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM5PR0802MB2611; 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)(3002001)(10201501046)(6055026); SRVR:AM5PR0802MB2611; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0802MB2611; x-forefront-prvs: 0052308DC6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(7916002)(377424004)(189002)(199003)(54534003)(101416001)(86362001)(10400500002)(33656002)(3660700001)(11100500001)(3280700002)(106356001)(106116001)(105586002)(2906002)(5660300001)(50986999)(76176999)(19580405001)(575784001)(2900100001)(2950100001)(19580395003)(54356999)(3846002)(76576001)(8936002)(81166006)(8676002)(110136002)(92566002)(81156014)(7696003)(7846002)(450100001)(305945005)(66066001)(74316002)(9686002)(77096005)(97736004)(87936001)(6116002)(68736007)(189998001)(122556002)(5002640100001)(7736002)(4326007)(102836003)(586003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0802MB2611; H:AM5PR0802MB2610.eurprd08.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Sep 2016 12:33:17.9896 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2611 X-MC-Unique: 2ly9MVcVOm6hjrAI8rDdjQ-1 Ping I noticed it would still be a good idea to add an extra barrier in the epilog as the scheduler doesn't appear to handle aliases of frame accesses properly. 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-08-10  Wilco Dijkstra  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. 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 5a25fce17785af9f9dc12e0f2a9609af09af0b35..bb8baff1e7a06942c8b8f51c1d6b341673401ef9 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))