This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, _eh, dawin, Version2] Allow targets to suppress epilogues in _eh frames.
- From: Jack Howarth <howarth at bromo dot med dot uc dot edu>
- To: IainS <developer at sandoe-acoustics dot co dot uk>
- Cc: Richard Henderson <rth at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, mrs at gcc dot gnu dot org
- Date: Wed, 15 Sep 2010 14:15:25 -0400
- Subject: Re: [Patch, _eh, dawin, Version2] Allow targets to suppress epilogues in _eh frames.
- 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> <FB9458D0-F758-483F-A22A-CB9F633EBC10@sandoe-acoustics.co.uk> <4C6AA346.3010501@redhat.com> <64A496AB-C878-42AD-85DE-7187118DD4AD@sandoe-acoustics.co.uk>
On Wed, Sep 15, 2010 at 03:23:08PM +0100, IainS wrote:
> Hi Richard,
>
> This kinda stalled ... I guess I should have pinged it.
>
> On 17 Aug 2010, at 15:57, Richard Henderson wrote:
>
>> On 08/17/2010 06:09 AM, IainS wrote:
>>> 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?
>>
>> Let's commit them as separate patches for bi-sect-ability,
>> Just In Case.
>
>>
>>> cfi->dw_cfi_opc = DW_CFA_GNU_args_size;
>>
>> Oh, that. If Darwin doesn't like that, then it's expecting the
>> compiler to always use -maccumulate-outgoing-args. Just make
>> sure that gets set by default. I suppose you'll have to do
>> something about the hand-full of test cases that force that
>> flag off.
>
>
> I believe that I've addressed the points made in the thread, is this now
> OK?
>
> I (re)-draw your attention to the fact that I've removed the
> unconditional setting of flag_asynchronous_unwind_tables when
> flag_non_call_exceptions is set in toplev.c.
>
> if this cannot be done then, perhaps, I need to return to the original
> idea of using a target hook to control the suppression of eh epilogues.
>
> thanks
> Iain
>
Iain,
Can you repost the patch as an attachment? Your mailer seems to be
corrupting the patch...
patching file gcc/dwarf2out.c
patch: **** malformed patch at line 73: /* Otherwise, search forward to see if the return insn was the last
Jack
> ----
>
> amended & refreshed patch:
>
> gcc:
>
> * gcc/dwarf2out.c: DW_CFA_INTERNAL_start_epilogue, CFI_T New.
> (struct dw_cfi_struct): Adjust type of dw_cfi_opc.
> (add_fde_cfi): Handle DW_CFA_INTERNAL_start_epilogue.
> (dwarf2out_cfi_begin_epilogue): Insert epilogue marker.
> (dw_cfi_oprnd1_desc): Adjust argument type.
> (dw_cfi_oprnd2_desc): Likewise.
> (output_cfi): Handle DW_CFA_INTERNAL_start_epilogue as nop.
> (output_cfi_directive): Likewise.
> (emit_cfi_or_skip_epilogue): New.
> (output_fde): Use emit_cfi_or_skip_epilogue.
> * gcc/toplev.c (process_options): Do not set
> flag_asynchronous_unwind_tables
> unless it is unset by the target. Set flag_unwind_tables for either
> flag_asynchronous_unwind_tables or flag_non_call_exceptions.
> * gcc/config/darwin.c (darwin_override_options): Warn the the target
> does
> not fully support flag_asynchronous_unwind_tables. Switch
> flag_unwind_tables
> on for flag_non_call_exceptions or flag_exceptions on darwin versions
> supporting
> _eh frames. Ensure that all table flags are switched off for kernel
> code.
> * gcc/config/i386/darwin.h (SUBTARGET_OVERRIDE_OPTIONS): New, handle
> making a default unwinder when none is specified by the user.
>
> testsuite:
>
> * g++.dg/eh/async-unwind1.C: Prune warning for Darwin.
> * g++.dg/eh/async-unwind2.C: Likewise.
>
>
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c (revision 164304)
> +++ 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)")))
> @@ -820,9 +832,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 ();
> @@ -2893,12 +2911,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
> @@ -2940,11 +2968,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)
> {
> @@ -2952,6 +2978,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:
> @@ -2989,11 +3016,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)
> {
> @@ -3120,6 +3146,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,
> @@ -3302,6 +3330,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;
> @@ -3497,6 +3529,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
> @@ -3612,13 +3682,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;
> }
> @@ -3626,7 +3696,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;
> @@ -3635,7 +3704,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 164304)
> +++ gcc/toplev.c (working copy)
> @@ -1881,9 +1884,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-unwind2.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind2.C (revision 164304)
> +++ 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 <stdarg.h>
>
> Index: gcc/testsuite/g++.dg/eh/async-unwind1.C
> ===================================================================
> --- gcc/testsuite/g++.dg/eh/async-unwind1.C (revision 164304)
> +++ 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/config/darwin.c
> ===================================================================
> --- gcc/config/darwin.c (revision 164304)
> +++ gcc/config/darwin.c (working copy)
> @@ -1869,11 +1900,25 @@ darwin_kextabi_p (void) {
> 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 +1930,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 +1946,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 164304)
> +++ gcc/config/i386/darwin.h (working copy)
> @@ -253,6 +253,39 @@ 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) \
> + flag_unwind_tables = 1; /* Emit Unwind tables. */ \
> + else \
> + { \
> + /* The user has asked to omit fp - emit Unwind tables. */ \
> + if (flag_omit_frame_pointer == 1) \
> + flag_unwind_tables = 1; \
> + else \
> + flag_omit_frame_pointer = 0;/* Use the frame pointer. */ \
> + } \
> + } \
> + /* Default frame pointers on for earlier versions of Darwin. */ \
> + if (flag_omit_frame_pointer == 2 && !darwin9plus) \
> + flag_omit_frame_pointer = 0; \
> + /* Do not set asynchronous_unwinding (yet) unless the user \
> + specifically asks. */ \
> + if (flag_asynchronous_unwind_tables == 2) \
> + flag_asynchronous_unwind_tables = 0; \
> + /* Jam this on to avoid a GNU-specific dwarf opcodeup setting \
> + Darwin10's eh frame compacter. */ \
> + if (flag_exceptions || flag_asynchronous_unwind_tables \
> + || flag_unwind_tables) \
> + target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS; \
> +} 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