From patchwork Thu Nov 2 12:00:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Claudiu Zissulescu X-Patchwork-Id: 833315 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-465733-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="xd4UiTAA"; dkim-atps=neutral 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 3ySP0P304Dz9t2f for ; Thu, 2 Nov 2017 23:04:52 +1100 (AEDT) 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:mime-version:content-type; q=dns; s=default; b=oL2DIyKp+rVA7wjtInaPs0Ua8hG1qfhW/IFcV8sqZHgfgMhxe/ aHwlLBSh2EmU94aycJbHvf4G1sDZEWU5KvNwJ7iAv5gMTvaDA5/snYje+BPb2mz0 nY1WJFmHM4WhyzQ5modTwMfy1dCNk13AtnUo2I1+MHdGbNI0LiXsmJBO8= 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:mime-version:content-type; s= default; bh=VrffTXZ1i/LmF+Fjo78tLLUnpbc=; b=xd4UiTAAg1TGgqRCGWSb zkSwxgXKzCUaWJb8wEP8UJH1BzZyQC0MVW0HmLYGerOcMOz2klBsJYuIgXgq7W3P pZWvsO+HlL/wJWnt2Pdv0cEZblXnjYJdSAH+eFG7HZNu07hmOegL/n1Wpk0I049G Y+5T35IOZokj5/g0Qh3yVB8= Received: (qmail 115544 invoked by alias); 2 Nov 2017 12:04:38 -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 115533 invoked by uid 89); 2 Nov 2017 12:04:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-22.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, UNSUBSCRIBE_BODY autolearn=ham version=3.3.2 spammy=H*RU:sk:smtprel, Hx-spam-relays-external:sk:smtprel, loaded, consecutive X-HELO: smtprelay.synopsys.com Received: from smtprelay.synopsys.com (HELO smtprelay.synopsys.com) (198.182.60.111) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 02 Nov 2017 12:04:26 +0000 Received: from mailhost.synopsys.com (mailhost2.synopsys.com [10.13.184.66]) by smtprelay.synopsys.com (Postfix) with ESMTP id 3746610C158A; Thu, 2 Nov 2017 05:04:25 -0700 (PDT) Received: from mailhost.synopsys.com (localhost [127.0.0.1]) by mailhost.synopsys.com (Postfix) with ESMTP id 1DA9CAF9; Thu, 2 Nov 2017 05:04:25 -0700 (PDT) Received: from US01WXQAHTC1.internal.synopsys.com (us01wxqahtc1.internal.synopsys.com [10.12.238.230]) by mailhost.synopsys.com (Postfix) with ESMTP id E4898AE6; Thu, 2 Nov 2017 05:04:20 -0700 (PDT) Received: from IN01WEHTCB.internal.synopsys.com (10.144.199.106) by US01WXQAHTC1.internal.synopsys.com (10.12.238.230) with Microsoft SMTP Server (TLS) id 14.3.266.1; Thu, 2 Nov 2017 05:04:00 -0700 Received: from IN01WEHTCA.internal.synopsys.com (10.144.199.103) by IN01WEHTCB.internal.synopsys.com (10.144.199.105) with Microsoft SMTP Server (TLS) id 14.3.266.1; Thu, 2 Nov 2017 17:33:59 +0530 Received: from nl20droid1.internal.synopsys.com (10.100.24.228) by IN01WEHTCA.internal.synopsys.com (10.144.199.243) with Microsoft SMTP Server (TLS) id 14.3.266.1; Thu, 2 Nov 2017 17:33:59 +0530 From: Claudiu Zissulescu To: CC: , , Subject: [PATCH] [ARC] Reimplement exception handling support. Date: Thu, 2 Nov 2017 13:00:09 +0100 Message-ID: <1509624009-21119-1-git-send-email-claziss@synopsys.com> MIME-Version: 1.0 This is patch which solves the ARC issues with exception handling support. Ok to apply? Claudiu 2016-06-09 Claudiu Zissulescu Andrew Burgess * config/arc/arc-protos.h (arc_compute_frame_size): Delete declaration. (arc_return_slot_offset): Likewise. (arc_eh_return_address_location): New declaration. * config/arc/arc.c (TARGET_BUILTIN_SETJMP_FRAME_VALUE): Define. (MUST_SAVE_REGISTER): Add exception handler case. (MUST_SAVE_RETURN_ADDR): Likewise. (arc_frame_pointer_required): Likewise. (arc_frame_pointer_needed): New function. (arc_compute_frame_size): Changed. (arc_expand_prologue): Likewise. (arc_expand_epilogue): Likewise. (arc_initial_elimination_offset): Likewise. (arc_return_slot_offset): Delete. (arc_eh_return_address_location): New function. (arc_builtin_setjmp_frame_value): Likewise. * config/arc/arc.h (EH_RETURN_DATA_REGNO): Use 2 registers. (EH_RETURN_STACKADJ_RTX): Define. (EH_RETURN_HANDLER_RTX): Likewise. * config/arc/arc.md (eh_return): Delete. --- gcc/config/arc/arc-protos.h | 2 +- gcc/config/arc/arc.c | 202 +++++++++++++++++++++++++++++++++++--------- gcc/config/arc/arc.h | 7 +- gcc/config/arc/arc.md | 33 -------- 4 files changed, 166 insertions(+), 78 deletions(-) diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h index 1c7031c..6e7239f 100644 --- a/gcc/config/arc/arc-protos.h +++ b/gcc/config/arc/arc-protos.h @@ -111,8 +111,8 @@ extern bool arc_epilogue_uses (int regno); extern bool arc_eh_uses (int regno); /* insn-attrtab.c doesn't include reload.h, which declares regno_clobbered_p. */ extern int regno_clobbered_p (unsigned int, rtx_insn *, machine_mode, int); -extern int arc_return_slot_offset (void); extern bool arc_legitimize_reload_address (rtx *, machine_mode, int, int); extern void arc_secondary_reload_conv (rtx, rtx, rtx, bool); extern void arc_cpu_cpp_builtins (cpp_reader *); extern bool arc_store_addr_hazard_p (rtx_insn *, rtx_insn *); +extern rtx arc_eh_return_address_location (void); diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c index a0b66758..75d35cd 100644 --- a/gcc/config/arc/arc.c +++ b/gcc/config/arc/arc.c @@ -597,6 +597,8 @@ static void arc_finalize_pic (void); #undef TARGET_MODES_TIEABLE_P #define TARGET_MODES_TIEABLE_P arc_modes_tieable_p +#undef TARGET_BUILTIN_SETJMP_FRAME_VALUE +#define TARGET_BUILTIN_SETJMP_FRAME_VALUE arc_builtin_setjmp_frame_value /* Try to keep the (mov:DF _, reg) as early as possible so that the dh-lr insns appear together and can @@ -2556,8 +2558,7 @@ arc_compute_function_type (struct function *fun) Addition for pic: The gp register needs to be saved if the current function changes it to access gotoff variables. FIXME: This will not be needed if we used some arbitrary register - instead of r26. -*/ + instead of r26. */ static bool arc_must_save_register (int regno, struct function *func) @@ -2620,14 +2621,51 @@ arc_must_save_return_addr (struct function *func) /* Helper function to wrap FRAME_POINTER_NEEDED. We do this as FRAME_POINTER_NEEDED will not be true until the IRA (Integrated Register Allocator) pass, while we want to get the frame size - correct earlier than the IRA pass. */ + correct earlier than the IRA pass. + + When a function uses eh_return we must ensure that the fp register + is saved and then restored so that the unwinder can restore the + correct value for the frame we are going to jump to. + + To do this we force all frames that call eh_return to require a + frame pointer (see changes to arc_frame_pointer_required), this + will ensure that the previous frame pointer is stored on entry to + the function, and will then be reloaded at function exit. + + As the frame pointer is handled as a special case in our prologue + and epilogue code it must not be saved and restored using the + MUST_SAVE_REGISTER mechanism otherwise we run into issues where GCC + believes that the function is not using a frame pointer and that + the value in the fp register is the frame pointer, while the + prologue and epilogue are busy saving and restoring the fp + register. This issue is fixed in this commit too. + + During compilation of a function the frame size is evaluated + multiple times, it is not until the reload pass is complete the the + frame size is considered fixed (it is at this point that space for + all spills has been allocated). However the frame_pointer_needed + variable is not set true until the register allocation pass, as a + result in the early stages the frame size does not include space + for the frame pointer to be spilled. + + The problem that this causes, that I have not yet tracked down, is + that the rtl generated for EH_RETURN_HANDLER_RTX uses the details + of the frame size to compute the offset from the frame pointer at + which the return address lives. However, in early passes GCC has + not yet realised we need a frame pointer, and so has not included + space for the frame pointer in the frame size, and so gets the + offset of the return address wrong. This should not be an issue as + in later passes GCC has realised that the frame pointer needs to be + spilled, and has increased the frame size. However, the rtl for + the EH_RETURN_HANDLER_RTX is not regenerated to use the newer, + larger offset, and the wrong smaller offset is used. */ + static bool arc_frame_pointer_needed (void) { - return (frame_pointer_needed); + return (frame_pointer_needed || crtl->calls_eh_return); } - /* Return non-zero if there are registers to be saved or loaded using millicode thunks. We can only use consecutive sequences starting with r13, and not going beyond r25. @@ -2658,26 +2696,32 @@ arc_compute_millicode_save_restore_regs (unsigned int gmask, return 0; } -/* Return the bytes needed to compute the frame pointer from the current - stack pointer. +/* Return the bytes needed to compute the frame pointer from the + current stack pointer. */ - SIZE is the size needed for local variables. */ - -unsigned int -arc_compute_frame_size (int size) /* size = # of var. bytes allocated. */ +static unsigned int +arc_compute_frame_size (void) { int regno; unsigned int total_size, var_size, args_size, pretend_size, extra_size; unsigned int reg_size, reg_offset; unsigned int gmask; - struct arc_frame_info *frame_info = &cfun->machine->frame_info; + enum arc_function_type fn_type; + int interrupt_p; + struct arc_frame_info *frame_info; + int size; + + /* The answer might already be known. */ + if (cfun->machine->frame_info.initialized) + return cfun->machine->frame_info.total_size; - size = ARC_STACK_ALIGN (size); + frame_info = &cfun->machine->frame_info; + size = ARC_STACK_ALIGN (get_frame_size ()); - /* 1) Size of locals and temporaries */ + /* 1) Size of locals and temporaries. */ var_size = size; - /* 2) Size of outgoing arguments */ + /* 2) Size of outgoing arguments. */ args_size = crtl->outgoing_args_size; /* 3) Calculate space needed for saved registers. @@ -2698,12 +2742,29 @@ arc_compute_frame_size (int size) /* size = # of var. bytes allocated. */ } } + /* In a frame that calls __builtin_eh_return two data registers are + used to pass values back to the exception handler. + + Ensure that these registers are spilled to the stack so that the + exception throw code can find them, and update the saved values. + The handling code will then consume these reloaded values to + handle the exception. */ + if (crtl->calls_eh_return) + for (regno = 0; EH_RETURN_DATA_REGNO (regno) != INVALID_REGNUM; regno++) + { + reg_size += UNITS_PER_WORD; + gmask |= 1 << regno; + } + /* 4) Space for back trace data structure. (if required) + (if required). */ frame_info->save_return_addr - = (!crtl->is_leaf || df_regs_ever_live_p (RETURN_ADDR_REGNUM)); + = (!crtl->is_leaf || df_regs_ever_live_p (RETURN_ADDR_REGNUM) + || crtl->calls_eh_return); /* Saving blink reg in case of leaf function for millicode thunk calls. */ - if (optimize_size && !TARGET_NO_MILLICODE_THUNK_SET) + if (optimize_size + && !TARGET_NO_MILLICODE_THUNK_SET + && !crtl->calls_eh_return) { if (arc_compute_millicode_save_restore_regs (gmask, frame_info)) frame_info->save_return_addr = true; @@ -2731,7 +2792,11 @@ arc_compute_frame_size (int size) /* size = # of var. bytes allocated. */ /* Compute total frame size. */ total_size = var_size + args_size + extra_size + pretend_size + reg_size; - total_size = ARC_STACK_ALIGN (total_size); + /* It used to be the case that the alignment was forced at this + point. However, that is dangerous, calculations based on + total_size would be wrong. Given that this has never cropped up + as an issue I've changed this to an assert for now. */ + gcc_assert (total_size == ARC_STACK_ALIGN (total_size)); /* Compute offset of register save area from stack pointer: Frame: pretend_size reg_size var_size args_size <--sp @@ -2999,7 +3064,7 @@ arc_dwarf_emit_irq_save_regs (void) void arc_expand_prologue (void) { - int size = get_frame_size (); + int size; unsigned int gmask = cfun->machine->frame_info.gmask; /* unsigned int frame_pointer_offset;*/ unsigned int frame_size_to_allocate; @@ -3013,12 +3078,8 @@ arc_expand_prologue (void) if (ARC_NAKED_P (fn_type)) return; - size = ARC_STACK_ALIGN (size); - - /* Compute/get total frame size. */ - size = (!cfun->machine->frame_info.initialized - ? arc_compute_frame_size (size) - : cfun->machine->frame_info.total_size); + /* Compute total frame size. */ + size = arc_compute_frame_size (); if (flag_stack_usage_info) current_function_static_stack_size = size; @@ -3121,13 +3182,10 @@ arc_expand_prologue (void) void arc_expand_epilogue (int sibcall_p) { - int size = get_frame_size (); + int size; unsigned int fn_type = arc_compute_function_type (cfun); - size = ARC_STACK_ALIGN (size); - size = (!cfun->machine->frame_info.initialized - ? arc_compute_frame_size (size) - : cfun->machine->frame_info.total_size); + size = arc_compute_frame_size (); unsigned int pretend_size = cfun->machine->frame_info.pretend_size; unsigned int frame_size; @@ -3295,21 +3353,59 @@ arc_expand_epilogue (int sibcall_p) if (size > restored) frame_stack_add (size - restored); + /* For frames that use __builtin_eh_return, the register defined by + EH_RETURN_STACKADJ_RTX is set to 0 for all standard return paths. + On eh_return paths however, the register is set to the value that + should be added to the stack pointer in order to restore the + correct stack pointer for the exception handling frame. + + For ARC we are going to use r2 for EH_RETURN_STACKADJ_RTX, add + this onto the stack for eh_return frames. */ + if (crtl->calls_eh_return) + emit_insn (gen_add2_insn (stack_pointer_rtx, + EH_RETURN_STACKADJ_RTX)); + /* Emit the return instruction. */ if (sibcall_p == FALSE) emit_jump_insn (gen_simple_return ()); } -/* Return the offset relative to the stack pointer where the return address - is stored, or -1 if it is not stored. */ - -int -arc_return_slot_offset () -{ - struct arc_frame_info *afi = &cfun->machine->frame_info; +/* Return rtx for the location of the return address on the stack, + suitable for use in __builtin_eh_return. The new return address + will be written to this location in order to redirect the return to + the exception handler. */ - return (afi->save_return_addr - ? afi->total_size - afi->pretend_size - afi->extra_size : -1); +rtx +arc_eh_return_address_location (void) +{ + rtx mem; + int offset; + struct arc_frame_info *afi; + + arc_compute_frame_size (); + afi = &cfun->machine->frame_info; + + gcc_assert (crtl->calls_eh_return); + gcc_assert (afi->save_return_addr); + gcc_assert (afi->extra_size >= 4); + + /* The '-4' removes the size of the return address, which is + included in the 'extra_size' field. */ + offset = afi->reg_size + afi->extra_size - 4; + mem = gen_frame_mem (Pmode, + plus_constant (Pmode, frame_pointer_rtx, offset)); + + /* The following should not be needed, and is, really a hack. The + issue being worked around here is that the DSE (Dead Store + Elimination) pass will remove this write to the stack as it sees + a single store and no corresponding read. The read however + occurs in the epilogue code, which is not added into the function + rtl until a later pass. So, at the time of DSE, the decision to + remove this store seems perfectly sensible. Marking the memory + address as volatile obviously has the effect of preventing DSE + from removing the store. */ + MEM_VOLATILE_P (mem) = 1; + return mem; } /* PIC */ @@ -4807,8 +4903,8 @@ arc_can_eliminate (const int from ATTRIBUTE_UNUSED, const int to) int arc_initial_elimination_offset (int from, int to) { - if (! cfun->machine->frame_info.initialized) - arc_compute_frame_size (get_frame_size ()); + if (!cfun->machine->frame_info.initialized) + arc_compute_frame_size (); if (from == ARG_POINTER_REGNUM && to == FRAME_POINTER_REGNUM) { @@ -4836,7 +4932,7 @@ arc_initial_elimination_offset (int from, int to) static bool arc_frame_pointer_required (void) { - return cfun->calls_alloca; + return cfun->calls_alloca || crtl->calls_eh_return; } @@ -10633,6 +10729,28 @@ compact_memory_operand_p (rtx op, machine_mode mode, return false; } +/* Return the frame pointer value to be backed up in the setjmp buffer. */ + +static rtx +arc_builtin_setjmp_frame_value (void) +{ + /* We always want to preserve whatever value is currently in the frame + pointer register. For frames that are using the frame pointer the new + value of the frame pointer register will have already been computed + (as part of the prologue). For frames that are not using the frame + pointer it is important that we backup whatever value is in the frame + pointer register, as earlier (more outer) frames may have placed a + value into the frame pointer register. It might be tempting to try + and use `frame_pointer_rtx` here, however, this is not what we want. + For frames that are using the frame pointer this will give the + correct value. However, for frames that are not using the frame + pointer this will still give the value that _would_ have been the + frame pointer value for this frame (if the use of the frame pointer + had not been removed). We really do want the raw frame pointer + register value. */ + return gen_raw_REG (Pmode, FRAME_POINTER_REGNUM); +} + /* Implement TARGET_USE_ANCHORS_FOR_SYMBOL_P. We don't want to use anchors for small data: the GP register acts as an anchor in that case. We also don't want to use them for PC-relative accesses, diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h index 1d9063a..b1458dc 100644 --- a/gcc/config/arc/arc.h +++ b/gcc/config/arc/arc.h @@ -1365,8 +1365,11 @@ do { \ /* Frame info. */ -#define EH_RETURN_DATA_REGNO(N) \ - ((N) < 4 ? (N) : INVALID_REGNUM) +#define EH_RETURN_DATA_REGNO(N) ((N) < 2 ? (N) : INVALID_REGNUM) + +#define EH_RETURN_STACKADJ_RTX gen_rtx_REG (Pmode, 2) + +#define EH_RETURN_HANDLER_RTX arc_eh_return_address_location () /* Turn off splitting of long stabs. */ #define DBX_CONTIN_LENGTH 0 diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md index c766306..faf6698 100644 --- a/gcc/config/arc/arc.md +++ b/gcc/config/arc/arc.md @@ -4935,39 +4935,6 @@ (const_int 4)] (const_int 2)))]) -(define_insn_and_split "eh_return" - [(eh_return) - (use (match_operand:SI 0 "move_src_operand" "rC32,mCalCpc")) - (clobber (match_scratch:SI 1 "=X,r")) - (clobber (match_scratch:SI 2 "=&r,r"))] - "" - "#" - "reload_completed" - [(set (match_dup 2) (match_dup 0))] -{ - int offs = arc_return_slot_offset (); - - if (offs < 0) - operands[2] = gen_rtx_REG (Pmode, RETURN_ADDR_REGNUM); - else - { - if (!register_operand (operands[0], Pmode) - && !satisfies_constraint_C32 (operands[0])) - { - emit_move_insn (operands[1], operands[0]); - operands[0] = operands[1]; - } - rtx addr = plus_constant (Pmode, stack_pointer_rtx, offs); - if (!strict_memory_address_p (Pmode, addr)) - { - emit_move_insn (operands[2], addr); - addr = operands[2]; - } - operands[2] = gen_frame_mem (Pmode, addr); - } -} - [(set_attr "length" "12")]) - ;; ??? #ifdefs in function.c require the presence of this pattern, with a ;; non-constant predicate. (define_expand "return"