From patchwork Tue Aug 17 13:09:08 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 61891 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]) by ozlabs.org (Postfix) with SMTP id 1CF22B70D2 for ; Tue, 17 Aug 2010 23:10:00 +1000 (EST) Received: (qmail 4424 invoked by alias); 17 Aug 2010 13:09:57 -0000 Received: (qmail 4038 invoked by uid 22791); 17 Aug 2010 13:09:52 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, TW_TR X-Spam-Check-By: sourceware.org Received: from c2bthomr06.btconnect.com (HELO c2bthomr06.btconnect.com) (213.123.20.124) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 17 Aug 2010 13:09:36 +0000 Received: from thor.office (host81-138-1-83.in-addr.btopenworld.com [81.138.1.83]) by c2bthomr06.btconnect.com with ESMTP id FZR21988; Tue, 17 Aug 2010 14:09:11 +0100 (BST) X-Mirapoint-IP-Reputation: reputation=Fair-1, source=Queried, refid=0001.0A0B0302.4C6A89F6.004E, actions=tag Cc: GCC Patches , mrs@gcc.gnu.org Message-Id: From: IainS To: Richard Henderson In-Reply-To: <4C697905.6010801@redhat.com> Mime-Version: 1.0 (Apple Message framework v936) Subject: Re: [Patch, _eh, dawin, Version2] Allow targets to suppress epilogues in _eh frames. Date: Tue, 17 Aug 2010 14:09:08 +0100 References: <89584BF2-10CD-45DE-A4A5-4C2EE43396B3@sandoe-acoustics.co.uk> <4C65D4E6.4030505@redhat.com> <6FABA122-72B5-4F9B-8573-FCFF1481700C@sandoe-acoustics.co.uk> <4C6964E6.9020204@redhat.com> <01A1DB79-3FD9-4246-91E3-2B6EFB592660@sandoe-acoustics.co.uk> <4C697905.6010801@redhat.com> X-IsSubscribed: yes 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 On 16 Aug 2010, at 18:44, Richard Henderson wrote: > On 08/16/2010 09:38 AM, IainS wrote: >> On 16 Aug 2010, at 17:18, Richard Henderson wrote: >> >> .... other points taken on board.... >> >>> Also, I'm noticing that dwarf2out.c uses >>> flag_asynchronous_unwind_tables in places where it really means >>> cfun->can_throw_non_call_exceptions. In fact, almost all >>> occurrences of f_a_u_t in dwarf2out.c are in error. The only >>> exception that I can see off-hand is in fde_needed_for_eh_p, which >>> would need an extra check vs cfun->can_throw_non_call_exceptions. >> >> How should we proceed on this? > Patch dwarf2out.c? How else? heh, I guess I meant was solving the problem you noticed above a pre- condition of applying the patch, or should they be considered separate problems? >> And also there is the remaining question about the GNU-specific code >> (which I fear will get re-enabled when the stuff above is fixed). > > What remaining question? What GNU-specific code? this.... static void dwarf2out_args_size (const char *label, HOST_WIDE_INT size) { dw_cfi_ref cfi; if (size == old_args_size) return; old_args_size = size; cfi = new_cfi (); cfi->dw_cfi_opc = DW_CFA_GNU_args_size; <<<============ cfi->dw_cfi_oprnd1.dw_cfi_offset = size; add_fde_cfi (label, cfi); } is a dwarf-2 target supposed to respond to this (i.e. we have a darwin bug if it doesn't work) or ... should there be a fall-back for dwarf-2 strict? ===== I'm attaching a consolidated patch which takes on board the comments you've made. It regtests on i686-darwin9 and java functions on darwin10 with it in place, the same patch also restores Java functionality on 4.5.2. (I've left two debug prints in place for now - accepting that they need to be removed before it's applied). I'll cross-reference this in PR41991 in case someone else wants to pick it up but, unfortunately, I don't have time to advance it at the moment. cheers, Iain Index: gcc/dwarf2out.c =================================================================== --- gcc/dwarf2out.c (revision 163302) +++ gcc/dwarf2out.c (working copy) @@ -268,9 +272,17 @@ typedef union GTY(()) dw_cfi_oprnd_struct { } dw_cfi_oprnd; +/* Use the first out-of-band opcode number as a marker for the start of + epilogues. */ +#define DW_CFA_INTERNAL_start_epilogue 0x100 +#define CFI_T enum dwarf_call_frame_info + +static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc (unsigned int cfi); +static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc (unsigned int cfi); + typedef struct GTY(()) dw_cfi_struct { dw_cfi_ref dw_cfi_next; - enum dwarf_call_frame_info dw_cfi_opc; + unsigned int dw_cfi_opc; dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd1_desc (%1.dw_cfi_opc)"))) dw_cfi_oprnd1; dw_cfi_oprnd GTY ((desc ("dw_cfi_oprnd2_desc (%1.dw_cfi_opc)"))) @@ -821,9 +833,15 @@ add_fde_cfi (const char *label, dw_cfi_ref cfi) } list_head = &cie_cfi_head; - - if (dwarf2out_do_cfi_asm ()) + + if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue) { + dw_fde_ref fde = current_fde (); + gcc_assert (fde != NULL); + list_head = &fde->dw_fde_cfi; + } + else if (dwarf2out_do_cfi_asm ()) + { if (label) { dw_fde_ref fde = current_fde (); @@ -2894,12 +2912,22 @@ dwarf2out_cfi_begin_epilogue (rtx insn) return; /* Otherwise, search forward to see if the return insn was the last - basic block of the function. If so, we don't need save/restore. */ + basic block of the function. If so, we don't need save/restore. + However, we do mark the position so that we can skip the epilogue + in _eh frames where required. */ gcc_assert (i != NULL); i = next_real_insn (i); if (i == NULL) - return; + { + dw_cfi_ref cfi_epi_start; + /* Emit a marker for the epilogue start. */ + cfi_epi_start = new_cfi (); + cfi_epi_start->dw_cfi_opc = DW_CFA_INTERNAL_start_epilogue; + add_fde_cfi ("", cfi_epi_start); + return; + } + /* Insert the restore before that next real insn in the stream, and before a potential NOTE_INSN_EPILOGUE_BEG -- we do need these notes to be properly nested. This should be after any label or alignment. This @@ -2941,11 +2969,9 @@ dwarf2out_frame_debug_restore_state (void) } /* Describe for the GTY machinery what parts of dw_cfi_oprnd1 are used. */ -static enum dw_cfi_oprnd_type dw_cfi_oprnd1_desc - (enum dwarf_call_frame_info cfi); static enum dw_cfi_oprnd_type -dw_cfi_oprnd1_desc (enum dwarf_call_frame_info cfi) +dw_cfi_oprnd1_desc (unsigned int cfi) { switch (cfi) { @@ -2953,6 +2979,7 @@ static enum dw_cfi_oprnd_type case DW_CFA_GNU_window_save: case DW_CFA_remember_state: case DW_CFA_restore_state: + case DW_CFA_INTERNAL_start_epilogue: return dw_cfi_oprnd_unused; case DW_CFA_set_loc: @@ -2990,11 +3017,9 @@ static enum dw_cfi_oprnd_type } /* Describe for the GTY machinery what parts of dw_cfi_oprnd2 are used. */ -static enum dw_cfi_oprnd_type dw_cfi_oprnd2_desc - (enum dwarf_call_frame_info cfi); static enum dw_cfi_oprnd_type -dw_cfi_oprnd2_desc (enum dwarf_call_frame_info cfi) +dw_cfi_oprnd2_desc (unsigned int cfi) { switch (cfi) { @@ -3090,6 +3115,7 @@ switch_to_frame_table_section (int for_eh, bool ba debug_frame_section = get_section (DEBUG_FRAME_SECTION, SECTION_DEBUG, NULL); switch_to_section (debug_frame_section); + TGT_ASM_DBG_SECTION_LABEL (debug_frame_section); } } @@ -3121,6 +3147,8 @@ output_cfi (dw_cfi_ref cfi, dw_fde_ref fde, int fo dw2_asm_output_data (1, (cfi->dw_cfi_opc | (r & 0x3f)), "DW_CFA_restore, column %#lx", r); } + else if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue) + ; /* This is a nop. */ else { dw2_asm_output_data (1, cfi->dw_cfi_opc, @@ -3303,6 +3331,10 @@ output_cfi_directive (dw_cfi_ref cfi) cfi->dw_cfi_oprnd1.dw_cfi_offset); break; + case DW_CFA_INTERNAL_start_epilogue: + ; /* nop. */ + break; + case DW_CFA_remember_state: fprintf (asm_out_file, "\t.cfi_remember_state\n"); break; @@ -3498,6 +3530,44 @@ output_cfis (dw_cfi_ref cfi, bool do_cfi_asm, dw_f } } +/* Output cfi skipping save/restore and epilogues in _eh frames + for targets that do not want them. */ + +static dw_cfi_ref +emit_cfi_or_skip_epilogue (dw_cfi_ref cfi, dw_fde_ref fde, bool for_eh) +{ + if (for_eh + && !flag_asynchronous_unwind_tables) + { + if (cfi->dw_cfi_opc == DW_CFA_remember_state) + { + if (flag_debug_asm) + fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START + " DW_CFA_remember/restore_state pair skipped\n",asm_out_file); + /* Skip to the restore, unless there's an error and we fall off + the end. */ + while (cfi->dw_cfi_next + && cfi->dw_cfi_opc != DW_CFA_restore_state) + cfi = cfi->dw_cfi_next; + return cfi; + } + if (cfi->dw_cfi_opc == DW_CFA_INTERNAL_start_epilogue) + { + if (flag_debug_asm) + fputs (ASM_COMMENT_START"\t\t\t"ASM_COMMENT_START + " DW_CFA_INTERNAL_start_epilogue\n",asm_out_file); + /* Skip to the end. */ + while (cfi->dw_cfi_next) + cfi = cfi->dw_cfi_next; + return cfi; + } + } + + /* If it's not a special case, then just carry on. */ + output_cfi (cfi, fde, for_eh); + return cfi; +} + /* Output one FDE. */ static void @@ -3613,13 +3683,13 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco fde->dw_fde_current_label = begin; if (!fde->dw_fde_switched_sections) for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next) - output_cfi (cfi, fde, for_eh); + cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh); else if (!second) { if (fde->dw_fde_switch_cfi) for (cfi = fde->dw_fde_cfi; cfi != NULL; cfi = cfi->dw_cfi_next) { - output_cfi (cfi, fde, for_eh); + cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh); if (cfi == fde->dw_fde_switch_cfi) break; } @@ -3627,7 +3697,6 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco else { dw_cfi_ref cfi_next = fde->dw_fde_cfi; - if (fde->dw_fde_switch_cfi) { cfi_next = fde->dw_fde_switch_cfi->dw_cfi_next; @@ -3636,7 +3705,7 @@ output_fde (dw_fde_ref fde, bool for_eh, bool seco fde->dw_fde_switch_cfi->dw_cfi_next = cfi_next; } for (cfi = cfi_next; cfi != NULL; cfi = cfi->dw_cfi_next) - output_cfi (cfi, fde, for_eh); + cfi = emit_cfi_or_skip_epilogue (cfi, fde, for_eh); } /* If we are to emit a ref/link from function bodies to their frame tables, Index: gcc/toplev.c =================================================================== --- gcc/toplev.c (revision 163302) +++ gcc/toplev.c (working copy) @@ -1798,9 +1801,9 @@ process_options (void) if (flag_rename_registers == AUTODETECT_VALUE) flag_rename_registers = flag_unroll_loops || flag_peel_loops; - if (flag_non_call_exceptions) + if (flag_non_call_exceptions && flag_asynchronous_unwind_tables == 2) flag_asynchronous_unwind_tables = 1; - if (flag_asynchronous_unwind_tables) + if (flag_asynchronous_unwind_tables || flag_non_call_exceptions) flag_unwind_tables = 1; if (flag_value_profile_transformations) Index: gcc/testsuite/g++.dg/eh/async-unwind1.C =================================================================== --- gcc/testsuite/g++.dg/eh/async-unwind1.C (revision 163302) +++ gcc/testsuite/g++.dg/eh/async-unwind1.C (working copy) @@ -1,6 +1,7 @@ // PR rtl-optimization/36419 // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } } // { dg-options "-Os -fasynchronous-unwind-tables -mpreferred-stack-boundary=4" } +// { dg-warning "this architecture does not fully support" "" { target *-*-darwin* } 0 } extern "C" void abort (); Index: gcc/testsuite/g++.dg/eh/async-unwind2.C =================================================================== --- gcc/testsuite/g++.dg/eh/async-unwind2.C (revision 163302) +++ gcc/testsuite/g++.dg/eh/async-unwind2.C (working copy) @@ -2,6 +2,7 @@ // { dg-do run { target { { i?86-*-* x86_64-*-* } && ilp32 } } } // { dg-require-effective-target fpic } // { dg-options "-Os -fasynchronous-unwind-tables -fpic -fno-inline" } +// { dg-warning "this architecture does not fully support" "" { target *-*-darwin* } 0 } #include Index: gcc/config/darwin.c =================================================================== --- gcc/config/darwin.c (revision 163302) +++ gcc/config/darwin.c (working copy) @@ -1866,14 +1897,30 @@ darwin_kextabi_p (void) { return flag_apple_kext; } +/* Invoked from SUBSUBTARGET_OVERRIDE_OPTIONS from rs6000.c and + i386.c, this is processed after any arch-specific overrides. */ void darwin_override_options (void) { + bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 0; + /* Don't emit DWARF3/4 unless specifically selected. This is a workaround for tool bugs. */ if (dwarf_strict < 0) dwarf_strict = 1; + /* Do not set asynchronous_unwinding (yet) unless the user specifically + asks, warn that it might not work (for Wall). */ + if (flag_asynchronous_unwind_tables) + { + if (flag_asynchronous_unwind_tables == 2) + flag_asynchronous_unwind_tables = 0; + else + warning (OPT_Wall, + "this architecture does not fully support" + " -fasynchronous-unwind-tables"); + } + /* Disable -freorder-blocks-and-partition for darwin_emit_unwind_label. */ if (flag_reorder_blocks_and_partition && (targetm.asm_out.emit_unwind_label == darwin_emit_unwind_label)) @@ -1885,6 +1932,10 @@ darwin_override_options (void) flag_reorder_blocks = 1; } + if (flag_non_call_exceptions == 1 + || flag_asynchronous_unwind_tables == 1) + flag_unwind_tables = 1; + if (flag_mkernel || flag_apple_kext) { /* -mkernel implies -fapple-kext for C++ */ @@ -1897,18 +1948,21 @@ darwin_override_options (void) flag_exceptions = 0; /* No -fnon-call-exceptions data in kexts. */ flag_non_call_exceptions = 0; + /* so no tables either.. */ + flag_unwind_tables = 0; + flag_asynchronous_unwind_tables = 0; /* We still need to emit branch islands for kernel context. */ darwin_emit_branch_islands = true; } + if (flag_var_tracking - && strverscmp (darwin_macosx_version_min, "10.5") >= 0 + && darwin9plus && debug_info_level >= DINFO_LEVEL_NORMAL && debug_hooks->var_location != do_nothing_debug_hooks.var_location) flag_var_tracking_uninit = 1; /* It is assumed that branch island stubs are needed for earlier systems. */ - if (darwin_macosx_version_min - && strverscmp (darwin_macosx_version_min, "10.5") < 0) + if (! darwin9plus) darwin_emit_branch_islands = true; } Index: gcc/config/i386/darwin.h =================================================================== --- gcc/config/i386/darwin.h (revision 163302) +++ gcc/config/i386/darwin.h (working copy) @@ -245,6 +245,41 @@ extern int darwin_emit_branch_islands; SUBTARGET_C_COMMON_OVERRIDE_OPTIONS; \ } while (0) +#undef SUBTARGET_OVERRIDE_OPTIONS +#define SUBTARGET_OVERRIDE_OPTIONS \ +do { \ + bool darwin9plus = strverscmp (darwin_macosx_version_min, "10.5") >= 0;\ + /* If no unwind model is set, then decide on a default based \ + on Darwin rev. */ \ + if (flag_omit_frame_pointer >= 1 && flag_unwind_tables == 0) \ + { \ + if (darwin9plus) \ + /* Emit Unwind tables. */ \ + flag_unwind_tables = 1; \ + else \ + { \ + if (flag_omit_frame_pointer == 1) \ + /* The user has asked to omit fp - emit Unwind tables. */ \ + flag_unwind_tables = 1; \ + else \ + /* Use the frame pointer. */ \ + flag_omit_frame_pointer = 0; \ + } \ + } \ + /* Do not allow the subtarget to switch off frame pointers for \ + earlier versions of Darwin. */ \ + if (flag_omit_frame_pointer == 2 && !darwin9plus) \ + flag_omit_frame_pointer = 0; \ + if (flag_omit_frame_pointer >= 1 && flag_unwind_tables == 1) \ + target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS; \ + /* Do not set asynchronous_unwinding (yet) unless the user \ + specifically asks,. */ \ + if (flag_asynchronous_unwind_tables == 2) \ + flag_asynchronous_unwind_tables = 0; \ + if (TARGET_64BIT && MACHO_DYNAMIC_NO_PIC_P) \ + target_flags &= ~MASK_MACHO_DYNAMIC_NO_PIC; \ +} while (0) + /* Darwin on x86_64 uses dwarf-2 by default. Pre-darwin9 32-bit compiles default to stabs+. darwin9+ defaults to dwarf-2. */ #ifndef DARWIN_PREFER_DWARF