| Message ID | 20170930090827.6604-6-aoliva@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9,SFN] adjust RTL insn-walking API | expand |
On Sat, Sep 30, 2017 at 11:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote: > This API change will enable final_start_function() to "consume" > initial insns, and choose the first insn to be passed to final(). > > Many ports call final_start_function() and final() when creating > thunks and whatnot, so they needed adjusting. But for MI thunks (and whatnot) there's no debug info. So why do we care? Most of them ask for sth very low-level. That is, almost all the time the sequence is final_start_function immediately followed by a call to final. So isn't better refactoring the answer here? Some ports only call final_start_function and never final for their thunks for example. That said, what do we lose when you do not adjust these things? After all final () does more stuff than calling final_scan_insn on all insns but you only do that for those you handle in final_start_function. It seems to me that the current split between start/final/end is too artificial and that the few "raw" building blocks that backends maybe need should be factored out for them? That said, it looks like an ugly "layering" violation you are introducing. But of course I don't know history or details around this part of the compiler... That said, all ports invoking final () invoke it in the start/final/end sequence. spu, s390, rs6000 (and thus powerpcspe), pa, nds32, i386, cris, arm invoke final_start_function and not final in the next 3 lines. They all invoke final_end_function but not final inbetween (all invokers of final do the 3-line dance). So, refactor final() to do all three and provide the chunks targets really care for? Why can't we do the param locview handling from final ()? Probably all remanents of targets with text prologues? Anyway, don't feel like acking in its current state because of this, maybe somebody else can chime in. Thanks, Richard. > for gcc/ChangeLog > > * output.h (final_start_function): Adjust. > * final.c (final_start_function): Take pointer to FIRST. > (rest_of_handle_final): Adjust. > * config/aarch64/aarch64.c (aarch64_output_mi_thunk): Adjust. > * config/alpha/alpha.c (alpha_output_mi_thunk_osf): Likewise. > * config/arm/arm.c (arm_thumb1_mi_thunk): Likewise. > (arm32_output_mi_thunk): Likewise. > * config/cris/cris.c (cris_asm_output_mi_thunk): Likewise. > * config/i386/i386.c (ix86_code_end): Likewise. > (x86_output_mi_thunk): Likewise. > * config/ia64/ia64.c (ia64_output_mi_thunk): Likewise. > * config/m68k/m68k.c (m68k_output_mi_thunk): Likewise. > * config/microblaze/microblaze.c (microblaze_asm_output_mi_thunk): > Likewise. > * config/mips/mips.c (mips_output_mi_thunk): Likewise. > * config/nds32/nds32.c (nds32_asm_output_mi_thunk): Likewise. > * config/nios2/nios2.c (nios2_asm_output_mi_thunk): Likewise. > * config/pa/pa.c (pa_asm_output_mi_thunk): Likewise. > * config/rs6000/rs6000.c (rs6000_output_mi_thunk): Likewise. > (rs6000_code_end): Likewise. > * config/s390/s390.c (s390_output_mi_thunk): Likewise. > * config/sh/sh.c (sh_output_mi_thunk): Likewise. > * config/sparc/sparc.c (sparc_output_mi_thunk): Likewise. > * config/spu/spu.c (spu_output_mi_thunk): Likewise. > * config/tilegx/tilegx.c (tilegx_output_mi_thunk): Likewise. > * config/tilepro/tilepro.c (tilepro_asm_output_mi_thunk): Likewise. > --- > gcc/config/aarch64/aarch64.c | 2 +- > gcc/config/alpha/alpha.c | 2 +- > gcc/config/arm/arm.c | 5 +++-- > gcc/config/cris/cris.c | 3 ++- > gcc/config/i386/i386.c | 5 +++-- > gcc/config/ia64/ia64.c | 2 +- > gcc/config/m68k/m68k.c | 2 +- > gcc/config/microblaze/microblaze.c | 2 +- > gcc/config/mips/mips.c | 2 +- > gcc/config/nds32/nds32.c | 3 ++- > gcc/config/nios2/nios2.c | 2 +- > gcc/config/pa/pa.c | 3 ++- > gcc/config/rs6000/rs6000.c | 5 +++-- > gcc/config/s390/s390.c | 3 ++- > gcc/config/sh/sh.c | 2 +- > gcc/config/sparc/sparc.c | 2 +- > gcc/config/spu/spu.c | 3 ++- > gcc/config/tilegx/tilegx.c | 2 +- > gcc/config/tilepro/tilepro.c | 2 +- > gcc/final.c | 9 ++++++--- > gcc/output.h | 2 +- > 21 files changed, 37 insertions(+), 26 deletions(-) > > diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > index 23f5aff..73872dd 100644 > --- a/gcc/config/aarch64/aarch64.c > +++ b/gcc/config/aarch64/aarch64.c > @@ -3961,7 +3961,7 @@ aarch64_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c > index 41f3e3a..56b6f04 100644 > --- a/gcc/config/alpha/alpha.c > +++ b/gcc/config/alpha/alpha.c > @@ -8480,7 +8480,7 @@ alpha_output_mi_thunk_osf (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > assemble_start_function and assemble_end_function. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > } > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index 4cddf3b..9301d58 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -26410,7 +26410,8 @@ arm_thumb1_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, > if (mi_delta < 0) > mi_delta = - mi_delta; > > - final_start_function (emit_barrier (), file, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, file, 1); > > if (TARGET_THUMB1) > { > @@ -26587,7 +26588,7 @@ arm32_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, > > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c > index fe80a27..3581d2d 100644 > --- a/gcc/config/cris/cris.c > +++ b/gcc/config/cris/cris.c > @@ -2755,7 +2755,8 @@ cris_asm_output_mi_thunk (FILE *stream, > tree funcdecl) > { > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), stream, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, stream, 1); > > if (delta > 0) > fprintf (stream, "\tadd%s " HOST_WIDE_INT_PRINT_DEC ",$%s\n", > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index 98fb1ce..d4d9490 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -12499,8 +12499,9 @@ ix86_code_end (void) > emitting it directly; tell them we're a thunk, if they care. */ > cfun->is_thunk = true; > first_function_block_is_cold = false; > + rtx_insn *first = emit_barrier (); > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), asm_out_file, 1); > + final_start_function (&first, asm_out_file, 1); > > /* Pad stack IP move with 4 instructions (two NOPs count > as one instruction). */ > @@ -43001,7 +43002,7 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, > Note that use_thunk calls assemble_start_function et al. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > } > diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c > index fce3006..a94ab67 100644 > --- a/gcc/config/ia64/ia64.c > +++ b/gcc/config/ia64/ia64.c > @@ -11035,7 +11035,7 @@ ia64_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > emit_all_insn_group_barriers (NULL); > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c > index cd2e15e..48b9c4a 100644 > --- a/gcc/config/m68k/m68k.c > +++ b/gcc/config/m68k/m68k.c > @@ -5142,7 +5142,7 @@ m68k_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > /* Run just enough of rest_of_compilation. */ > insn = get_insns (); > split_all_insns_noflow (); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c > index 53ca016..4ebe023 100644 > --- a/gcc/config/microblaze/microblaze.c > +++ b/gcc/config/microblaze/microblaze.c > @@ -3257,7 +3257,7 @@ microblaze_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > "borrowed" from rs6000.c. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c > index 4133375..d1f7bd8 100644 > --- a/gcc/config/mips/mips.c > +++ b/gcc/config/mips/mips.c > @@ -19394,7 +19394,7 @@ mips_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > split_all_insns_noflow (); > mips16_lay_out_constants (true); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/nds32/nds32.c b/gcc/config/nds32/nds32.c > index 65095ff..f6d5f06 100644 > --- a/gcc/config/nds32/nds32.c > +++ b/gcc/config/nds32/nds32.c > @@ -1633,7 +1633,8 @@ nds32_asm_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > int this_regno; > > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), file, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, file, 1); > > this_regno = (aggregate_value_p (TREE_TYPE (TREE_TYPE (function)), function) > ? 1 > diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c > index 2602605..6ab2c24 100644 > --- a/gcc/config/nios2/nios2.c > +++ b/gcc/config/nios2/nios2.c > @@ -4061,7 +4061,7 @@ nios2_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > assemble_start_function and assemble_end_function. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c > index 5e945fc..418a017 100644 > --- a/gcc/config/pa/pa.c > +++ b/gcc/config/pa/pa.c > @@ -8404,7 +8404,8 @@ pa_asm_output_mi_thunk (FILE *file, tree thunk_fndecl, HOST_WIDE_INT delta, > xoperands[1] = XEXP (DECL_RTL (thunk_fndecl), 0); > xoperands[2] = GEN_INT (delta); > > - final_start_function (emit_barrier (), file, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, file, 1); > > /* Output the thunk. We know that the function is in the same > translation unit (i.e., the same space) as the thunk, and that > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c > index 1e794a0..cf1a7bf 100644 > --- a/gcc/config/rs6000/rs6000.c > +++ b/gcc/config/rs6000/rs6000.c > @@ -29699,7 +29699,7 @@ rs6000_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > assemble_start_function and assemble_end_function. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > @@ -38165,7 +38165,8 @@ rs6000_code_end (void) > init_function_start (decl); > first_function_block_is_cold = false; > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), asm_out_file, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, asm_out_file, 1); > > fputs ("\tblr\n", asm_out_file); > > diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c > index 52a82df..0bea709 100644 > --- a/gcc/config/s390/s390.c > +++ b/gcc/config/s390/s390.c > @@ -13202,7 +13202,8 @@ s390_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > int nonlocal = 0; > > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), file, 1); > + rtx_insn *first = emit_barrier (); > + final_start_function (&first, file, 1); > > /* Operand 0 is the target function. */ > op[0] = XEXP (DECL_RTL (function), 0); > diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c > index 3c6d525..284fb81 100644 > --- a/gcc/config/sh/sh.c > +++ b/gcc/config/sh/sh.c > @@ -10936,7 +10936,7 @@ sh_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > > sh_reorg (); > shorten_branches (insns); > - final_start_function (insns, file, 1); > + final_start_function (&insns, file, 1); > final (insns, file, 1); > final_end_function (); > > diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c > index d3f002d..9432a66 100644 > --- a/gcc/config/sparc/sparc.c > +++ b/gcc/config/sparc/sparc.c > @@ -12140,7 +12140,7 @@ sparc_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > assemble_start_function and assemble_end_function. */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c > index b9af9a9..d22cf3e 100644 > --- a/gcc/config/spu/spu.c > +++ b/gcc/config/spu/spu.c > @@ -7045,7 +7045,8 @@ spu_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, > rtx op[8]; > > /* Make sure unwind info is emitted for the thunk if needed. */ > - final_start_function (emit_barrier (), file, 1); > + rtx_insn *insn = emit_barrier (); > + final_start_function (&insn, file, 1); > > /* Operand 0 is the target function. */ > op[0] = XEXP (DECL_RTL (function), 0); > diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c > index 63fe340..861577eca 100644 > --- a/gcc/config/tilegx/tilegx.c > +++ b/gcc/config/tilegx/tilegx.c > @@ -4998,7 +4998,7 @@ tilegx_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/config/tilepro/tilepro.c b/gcc/config/tilepro/tilepro.c > index ee9bc0a..e261400 100644 > --- a/gcc/config/tilepro/tilepro.c > +++ b/gcc/config/tilepro/tilepro.c > @@ -4421,7 +4421,7 @@ tilepro_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, > */ > insn = get_insns (); > shorten_branches (insn); > - final_start_function (insn, file, 1); > + final_start_function (&insn, file, 1); > final (insn, file, 1); > final_end_function (); > > diff --git a/gcc/final.c b/gcc/final.c > index 49cfbfb..d2b8523d 100644 > --- a/gcc/final.c > +++ b/gcc/final.c > @@ -1763,9 +1763,11 @@ get_some_local_dynamic_name () > test and compare insns. */ > > void > -final_start_function (rtx_insn *first, FILE *file, > +final_start_function (rtx_insn **firstp, FILE *file, > int optimize_p ATTRIBUTE_UNUSED) > { > + rtx_insn *first = *firstp; > + > block_depth = 0; > > this_is_asm_operands = 0; > @@ -4536,8 +4538,9 @@ rest_of_handle_final (void) > variable_tracking_main (); > > assemble_start_function (current_function_decl, fnname); > - final_start_function (get_insns (), asm_out_file, optimize); > - final (get_insns (), asm_out_file, optimize); > + rtx_insn *first = get_insns (); > + final_start_function (&first, asm_out_file, optimize); > + final (first, asm_out_file, optimize); > if (flag_ipa_ra > && !lookup_attribute ("noipa", DECL_ATTRIBUTES (current_function_decl))) > collect_fn_hard_reg_usage (); > diff --git a/gcc/output.h b/gcc/output.h > index e98a911..62a405d 100644 > --- a/gcc/output.h > +++ b/gcc/output.h > @@ -59,7 +59,7 @@ const char *get_some_local_dynamic_name (); > for the new function. The label for the function and associated > assembler pseudo-ops have already been output in > `assemble_start_function'. */ > -extern void final_start_function (rtx_insn *, FILE *, int); > +extern void final_start_function (rtx_insn **, FILE *, int); > > /* Output assembler code for the end of a function. > For clarity, args are same as those of `final_start_function' > -- > 2.9.5 >
On 10/19/2017 05:01 AM, Richard Biener wrote: > On Sat, Sep 30, 2017 at 11:08 AM, Alexandre Oliva <aoliva@redhat.com> wrote: >> This API change will enable final_start_function() to "consume" >> initial insns, and choose the first insn to be passed to final(). >> >> Many ports call final_start_function() and final() when creating >> thunks and whatnot, so they needed adjusting. > > But for MI thunks (and whatnot) there's no debug info. So why do we care? > Most of them ask for sth very low-level. True. > > That is, almost all the time the sequence is final_start_function immediately > followed by a call to final. So isn't better refactoring the answer here? > Some ports only call final_start_function and never final for their thunks > for example. > > That said, what do we lose when you do not adjust these things? Good question. And more generally when do we want to be able to do something sensible of this kind of glue code and what *is* sensible to do here? > > After all final () does more stuff than calling final_scan_insn on all > insns but you only do that for those you handle in final_start_function. > > It seems to me that the current split between start/final/end is too > artificial and that the few "raw" building blocks that backends maybe > need should be factored out for them? Perhaps. I think we'd need to see this fleshed out a bit more. > > That said, it looks like an ugly "layering" violation you are introducing. > > But of course I don't know history or details around this part of the > compiler... > > That said, all ports invoking final () invoke it in the > start/final/end sequence. > > spu, s390, rs6000 (and thus powerpcspe), pa, nds32, i386, cris, arm > invoke final_start_function and not final in the next 3 lines. They all > invoke final_end_function but not final inbetween (all invokers of final > do the 3-line dance). For the PA, thunks are emitted as pure assembly without generating RTL for the thunk. There is no RTL to pass to final (or if there is RTL lying around, it's probably not the thunk). There's a few things in the thunk that I don't think we can represent in RTL, though obviously a few UNSPECs could do the trick. One question in my mind is whether or not we can reasonably expect Alex to do that work, then the analysis on the other ports and fix them as well. Jeff
On 09/30/2017 03:08 AM, Alexandre Oliva wrote: > This API change will enable final_start_function() to "consume" > initial insns, and choose the first insn to be passed to final(). > > Many ports call final_start_function() and final() when creating > thunks and whatnot, so they needed adjusting. So I haven't really followed the discussion until now. What's driving the need to have some insns "consumed" and have more control over what tthe first insn passed to final will be? jeff
On Oct 31, 2017, Jeff Law <law@redhat.com> wrote: > On 09/30/2017 03:08 AM, Alexandre Oliva wrote: >> This API change will enable final_start_function() to "consume" >> initial insns, and choose the first insn to be passed to final(). >> >> Many ports call final_start_function() and final() when creating >> thunks and whatnot, so they needed adjusting. > So I haven't really followed the discussion until now. What's driving > the need to have some insns "consumed" and have more control over what > tthe first insn passed to final will be? We want to build debug notes that bind arguments into the initial view in a function. That initial view (first .loc note) is emitted in final_start_function. So we don't want to process the initial debug bind insns in final_start_function, and not process them again in final. In response to richi's objections, I reverted the API exposed by final.c so that we process the loc notes in final_start_function, and just skip them in final, so that no changes are required to the various backends, at a very slight performance penalty as the leading debug insns will be looked at twice instead of just once, when final is so used by the backends. As for uses within final.c, those benefit from an API change internal to that file, that allows the leading debug insns to be processed just once. Here are the relevant snippets from the updated patchset (yet to be posted): +/* We want to emit param bindings (before the first begin_stmt) in the + initial view, if we are emitting views. To that end, we may + consume initial notes in the function, processing them in + final_start_function, before signaling the beginning of the + prologue, rather than in final. + + We don't test whether the DECLs are PARM_DECLs: the assumption is + that there will be a NOTE_INSN_BEGIN_STMT marker before any + non-parameter NOTE_INSN_VAR_LOCATION. It's ok if the marker is not + there, we'll just have more variable locations bound in the initial + view, which is consistent with their being bound without any code + that would give them a value. */ + +static inline bool +in_initial_view_p (rtx_insn *insn) +{ + return !DECL_IGNORED_P (current_function_decl) + && debug_variable_location_views + && insn && GET_CODE (insn) == NOTE + && NOTE_KIND (insn) == NOTE_INSN_VAR_LOCATION; +} + /* Output assembler code for the start of a function, and initialize some of the variables in this file for the new function. The label for the function and associated @@ -1757,12 +1819,15 @@ get_some_local_dynamic_name () FIRST is the first insn of the rtl for the function being compiled. FILE is the file to write assembler code to. + SEEN should be initially set to zero, and it may be updated to + indicate we have references to the next location view, that would + require us to emit it at the current PC. OPTIMIZE_P is nonzero if we should eliminate redundant test and compare insns. */ -void -final_start_function (rtx_insn *first, FILE *file, - int optimize_p ATTRIBUTE_UNUSED) +static void +final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen, + int optimize_p ATTRIBUTE_UNUSED) { block_depth = 0; @@ -1780,8 +1845,21 @@ final_start_function (rtx_insn *first, FILE *file, if (flag_sanitize & SANITIZE_ADDRESS) asan_function_start (); + rtx_insn *first = *firstp; + if (in_initial_view_p (first)) + { + do + { + final_scan_insn (first, file, 0, 0, seen); + first = NEXT_INSN (first); + } + while (in_initial_view_p (first)); + *firstp = first; + } + if (!DECL_IGNORED_P (current_function_decl)) @@ -1856,6 +1934,17 @@ final_start_function (rtx_insn *first, FILE *file, profile_after_prologue (file); } +/* This is an exported final_start_function_1, callable without SEEN. */ + +void +final_start_function (rtx_insn *first, FILE *file, + int optimize_p ATTRIBUTE_UNUSED) +{ + int seen = 0; + final_start_function_1 (&first, file, &seen, optimize_p); + gcc_assert (seen == 0); +} + static void profile_after_prologue (FILE *file ATTRIBUTE_UNUSED) { @@ -1987,11 +2076,10 @@ dump_basic_block_info (FILE *file, rtx_insn *insn, basic_block *start_to_bb, /* Output assembler code for some insns: all or part of a function. For description of args, see `final_start_function', above. */ -void -final (rtx_insn *first, FILE *file, int optimize_p) +static void +final_1 (rtx_insn *first, FILE *file, int seen, int optimize_p) { rtx_insn *insn, *next; - int seen = 0; /* Used for -dA dump. */ basic_block *start_to_bb = NULL; @@ -2074,6 +2164,23 @@ final (rtx_insn *first, FILE *file, int optimize_p) delete_insn (insn); } } + +/* This is an exported final_1, callable without SEEN. */ + +void +final (rtx_insn *first, FILE *file, int optimize_p) +{ + /* Those that use the internal final_start_function_1/final_1 API + skip initial debug bind notes in final_start_function_1, and pass + the modified FIRST to final_1. But those that use the public + final_start_function/final APIs, final_start_function can't move + FIRST because it's not passed by reference, so if they were + skipped there, skip them again here. */ + while (in_initial_view_p (first)) + first = NEXT_INSN (first); + + final_1 (first, file, 0, optimize_p); +} const char * get_insn_template (int code, rtx insn) @@ -4525,8 +4650,10 @@ rest_of_handle_final (void) variable_tracking_main (); assemble_start_function (current_function_decl, fnname); - final_start_function (get_insns (), asm_out_file, optimize); - final (get_insns (), asm_out_file, optimize); + rtx_insn *first = get_insns (); + int seen = 0; + final_start_function_1 (&first, asm_out_file, &seen, optimize); + final_1 (first, asm_out_file, seen, optimize); if (flag_ipa_ra && !lookup_attribute ("noipa", DECL_ATTRIBUTES (current_function_decl))) collect_fn_hard_reg_usage ();
On Wed, Nov 1, 2017 at 7:18 PM, Alexandre Oliva <aoliva@redhat.com> wrote: > On Oct 31, 2017, Jeff Law <law@redhat.com> wrote: > >> On 09/30/2017 03:08 AM, Alexandre Oliva wrote: >>> This API change will enable final_start_function() to "consume" >>> initial insns, and choose the first insn to be passed to final(). >>> >>> Many ports call final_start_function() and final() when creating >>> thunks and whatnot, so they needed adjusting. >> So I haven't really followed the discussion until now. What's driving >> the need to have some insns "consumed" and have more control over what >> tthe first insn passed to final will be? > > We want to build debug notes that bind arguments into the initial view > in a function. That initial view (first .loc note) is emitted in > final_start_function. So we don't want to process the initial debug > bind insns in final_start_function, and not process them again in final. > > In response to richi's objections, I reverted the API exposed by final.c > so that we process the loc notes in final_start_function, and just skip > them in final, so that no changes are required to the various backends, > at a very slight performance penalty as the leading debug insns will be > looked at twice instead of just once, when final is so used by the > backends. That works for me - we can still improve with some refactoring but didn't introduce some ugliness in the way. Richard. > As for uses within final.c, those benefit from an API change internal to > that file, that allows the leading debug insns to be processed just > once. Here are the relevant snippets from the updated patchset (yet to > be posted): > > > +/* We want to emit param bindings (before the first begin_stmt) in the > + initial view, if we are emitting views. To that end, we may > + consume initial notes in the function, processing them in > + final_start_function, before signaling the beginning of the > + prologue, rather than in final. > + > + We don't test whether the DECLs are PARM_DECLs: the assumption is > + that there will be a NOTE_INSN_BEGIN_STMT marker before any > + non-parameter NOTE_INSN_VAR_LOCATION. It's ok if the marker is not > + there, we'll just have more variable locations bound in the initial > + view, which is consistent with their being bound without any code > + that would give them a value. */ > + > +static inline bool > +in_initial_view_p (rtx_insn *insn) > +{ > + return !DECL_IGNORED_P (current_function_decl) > + && debug_variable_location_views > + && insn && GET_CODE (insn) == NOTE > + && NOTE_KIND (insn) == NOTE_INSN_VAR_LOCATION; > +} > + > /* Output assembler code for the start of a function, > and initialize some of the variables in this file > for the new function. The label for the function and associated > @@ -1757,12 +1819,15 @@ get_some_local_dynamic_name () > > FIRST is the first insn of the rtl for the function being compiled. > FILE is the file to write assembler code to. > + SEEN should be initially set to zero, and it may be updated to > + indicate we have references to the next location view, that would > + require us to emit it at the current PC. > OPTIMIZE_P is nonzero if we should eliminate redundant > test and compare insns. */ > > -void > -final_start_function (rtx_insn *first, FILE *file, > - int optimize_p ATTRIBUTE_UNUSED) > +static void > +final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen, > + int optimize_p ATTRIBUTE_UNUSED) > { > block_depth = 0; > > @@ -1780,8 +1845,21 @@ final_start_function (rtx_insn *first, FILE *file, > if (flag_sanitize & SANITIZE_ADDRESS) > asan_function_start (); > > + rtx_insn *first = *firstp; > + if (in_initial_view_p (first)) > + { > + do > + { > + final_scan_insn (first, file, 0, 0, seen); > + first = NEXT_INSN (first); > + } > + while (in_initial_view_p (first)); > + *firstp = first; > + } > + > if (!DECL_IGNORED_P (current_function_decl)) > @@ -1856,6 +1934,17 @@ final_start_function (rtx_insn *first, FILE *file, > profile_after_prologue (file); > } > > +/* This is an exported final_start_function_1, callable without SEEN. */ > + > +void > +final_start_function (rtx_insn *first, FILE *file, > + int optimize_p ATTRIBUTE_UNUSED) > +{ > + int seen = 0; > + final_start_function_1 (&first, file, &seen, optimize_p); > + gcc_assert (seen == 0); > +} > + > static void > profile_after_prologue (FILE *file ATTRIBUTE_UNUSED) > { > @@ -1987,11 +2076,10 @@ dump_basic_block_info (FILE *file, rtx_insn *insn, basic_block *start_to_bb, > /* Output assembler code for some insns: all or part of a function. > For description of args, see `final_start_function', above. */ > > -void > -final (rtx_insn *first, FILE *file, int optimize_p) > +static void > +final_1 (rtx_insn *first, FILE *file, int seen, int optimize_p) > { > rtx_insn *insn, *next; > - int seen = 0; > > /* Used for -dA dump. */ > basic_block *start_to_bb = NULL; > @@ -2074,6 +2164,23 @@ final (rtx_insn *first, FILE *file, int optimize_p) > delete_insn (insn); > } > } > + > +/* This is an exported final_1, callable without SEEN. */ > + > +void > +final (rtx_insn *first, FILE *file, int optimize_p) > +{ > + /* Those that use the internal final_start_function_1/final_1 API > + skip initial debug bind notes in final_start_function_1, and pass > + the modified FIRST to final_1. But those that use the public > + final_start_function/final APIs, final_start_function can't move > + FIRST because it's not passed by reference, so if they were > + skipped there, skip them again here. */ > + while (in_initial_view_p (first)) > + first = NEXT_INSN (first); > + > + final_1 (first, file, 0, optimize_p); > +} > > const char * > get_insn_template (int code, rtx insn) > > > @@ -4525,8 +4650,10 @@ rest_of_handle_final (void) > variable_tracking_main (); > > assemble_start_function (current_function_decl, fnname); > - final_start_function (get_insns (), asm_out_file, optimize); > - final (get_insns (), asm_out_file, optimize); > + rtx_insn *first = get_insns (); > + int seen = 0; > + final_start_function_1 (&first, asm_out_file, &seen, optimize); > + final_1 (first, asm_out_file, seen, optimize); > if (flag_ipa_ra > && !lookup_attribute ("noipa", DECL_ATTRIBUTES (current_function_decl))) > collect_fn_hard_reg_usage (); > > -- > Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/ > You must be the change you wish to see in the world. -- Gandhi > Be Free! -- http://FSFLA.org/ FSF Latin America board member > Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 23f5aff..73872dd 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -3961,7 +3961,7 @@ aarch64_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/alpha/alpha.c b/gcc/config/alpha/alpha.c index 41f3e3a..56b6f04 100644 --- a/gcc/config/alpha/alpha.c +++ b/gcc/config/alpha/alpha.c @@ -8480,7 +8480,7 @@ alpha_output_mi_thunk_osf (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, assemble_start_function and assemble_end_function. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); } diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 4cddf3b..9301d58 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -26410,7 +26410,8 @@ arm_thumb1_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, if (mi_delta < 0) mi_delta = - mi_delta; - final_start_function (emit_barrier (), file, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, file, 1); if (TARGET_THUMB1) { @@ -26587,7 +26588,7 @@ arm32_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c index fe80a27..3581d2d 100644 --- a/gcc/config/cris/cris.c +++ b/gcc/config/cris/cris.c @@ -2755,7 +2755,8 @@ cris_asm_output_mi_thunk (FILE *stream, tree funcdecl) { /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), stream, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, stream, 1); if (delta > 0) fprintf (stream, "\tadd%s " HOST_WIDE_INT_PRINT_DEC ",$%s\n", diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 98fb1ce..d4d9490 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -12499,8 +12499,9 @@ ix86_code_end (void) emitting it directly; tell them we're a thunk, if they care. */ cfun->is_thunk = true; first_function_block_is_cold = false; + rtx_insn *first = emit_barrier (); /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), asm_out_file, 1); + final_start_function (&first, asm_out_file, 1); /* Pad stack IP move with 4 instructions (two NOPs count as one instruction). */ @@ -43001,7 +43002,7 @@ x86_output_mi_thunk (FILE *file, tree, HOST_WIDE_INT delta, Note that use_thunk calls assemble_start_function et al. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); } diff --git a/gcc/config/ia64/ia64.c b/gcc/config/ia64/ia64.c index fce3006..a94ab67 100644 --- a/gcc/config/ia64/ia64.c +++ b/gcc/config/ia64/ia64.c @@ -11035,7 +11035,7 @@ ia64_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, emit_all_insn_group_barriers (NULL); insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c index cd2e15e..48b9c4a 100644 --- a/gcc/config/m68k/m68k.c +++ b/gcc/config/m68k/m68k.c @@ -5142,7 +5142,7 @@ m68k_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, /* Run just enough of rest_of_compilation. */ insn = get_insns (); split_all_insns_noflow (); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/microblaze/microblaze.c b/gcc/config/microblaze/microblaze.c index 53ca016..4ebe023 100644 --- a/gcc/config/microblaze/microblaze.c +++ b/gcc/config/microblaze/microblaze.c @@ -3257,7 +3257,7 @@ microblaze_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, "borrowed" from rs6000.c. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 4133375..d1f7bd8 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -19394,7 +19394,7 @@ mips_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, split_all_insns_noflow (); mips16_lay_out_constants (true); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/nds32/nds32.c b/gcc/config/nds32/nds32.c index 65095ff..f6d5f06 100644 --- a/gcc/config/nds32/nds32.c +++ b/gcc/config/nds32/nds32.c @@ -1633,7 +1633,8 @@ nds32_asm_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, int this_regno; /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), file, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, file, 1); this_regno = (aggregate_value_p (TREE_TYPE (TREE_TYPE (function)), function) ? 1 diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c index 2602605..6ab2c24 100644 --- a/gcc/config/nios2/nios2.c +++ b/gcc/config/nios2/nios2.c @@ -4061,7 +4061,7 @@ nios2_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, assemble_start_function and assemble_end_function. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c index 5e945fc..418a017 100644 --- a/gcc/config/pa/pa.c +++ b/gcc/config/pa/pa.c @@ -8404,7 +8404,8 @@ pa_asm_output_mi_thunk (FILE *file, tree thunk_fndecl, HOST_WIDE_INT delta, xoperands[1] = XEXP (DECL_RTL (thunk_fndecl), 0); xoperands[2] = GEN_INT (delta); - final_start_function (emit_barrier (), file, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, file, 1); /* Output the thunk. We know that the function is in the same translation unit (i.e., the same space) as the thunk, and that diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 1e794a0..cf1a7bf 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -29699,7 +29699,7 @@ rs6000_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, assemble_start_function and assemble_end_function. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); @@ -38165,7 +38165,8 @@ rs6000_code_end (void) init_function_start (decl); first_function_block_is_cold = false; /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), asm_out_file, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, asm_out_file, 1); fputs ("\tblr\n", asm_out_file); diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c index 52a82df..0bea709 100644 --- a/gcc/config/s390/s390.c +++ b/gcc/config/s390/s390.c @@ -13202,7 +13202,8 @@ s390_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, int nonlocal = 0; /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), file, 1); + rtx_insn *first = emit_barrier (); + final_start_function (&first, file, 1); /* Operand 0 is the target function. */ op[0] = XEXP (DECL_RTL (function), 0); diff --git a/gcc/config/sh/sh.c b/gcc/config/sh/sh.c index 3c6d525..284fb81 100644 --- a/gcc/config/sh/sh.c +++ b/gcc/config/sh/sh.c @@ -10936,7 +10936,7 @@ sh_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, sh_reorg (); shorten_branches (insns); - final_start_function (insns, file, 1); + final_start_function (&insns, file, 1); final (insns, file, 1); final_end_function (); diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c index d3f002d..9432a66 100644 --- a/gcc/config/sparc/sparc.c +++ b/gcc/config/sparc/sparc.c @@ -12140,7 +12140,7 @@ sparc_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, assemble_start_function and assemble_end_function. */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/spu/spu.c b/gcc/config/spu/spu.c index b9af9a9..d22cf3e 100644 --- a/gcc/config/spu/spu.c +++ b/gcc/config/spu/spu.c @@ -7045,7 +7045,8 @@ spu_output_mi_thunk (FILE *file, tree thunk ATTRIBUTE_UNUSED, rtx op[8]; /* Make sure unwind info is emitted for the thunk if needed. */ - final_start_function (emit_barrier (), file, 1); + rtx_insn *insn = emit_barrier (); + final_start_function (&insn, file, 1); /* Operand 0 is the target function. */ op[0] = XEXP (DECL_RTL (function), 0); diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c index 63fe340..861577eca 100644 --- a/gcc/config/tilegx/tilegx.c +++ b/gcc/config/tilegx/tilegx.c @@ -4998,7 +4998,7 @@ tilegx_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/config/tilepro/tilepro.c b/gcc/config/tilepro/tilepro.c index ee9bc0a..e261400 100644 --- a/gcc/config/tilepro/tilepro.c +++ b/gcc/config/tilepro/tilepro.c @@ -4421,7 +4421,7 @@ tilepro_asm_output_mi_thunk (FILE *file, tree thunk_fndecl ATTRIBUTE_UNUSED, */ insn = get_insns (); shorten_branches (insn); - final_start_function (insn, file, 1); + final_start_function (&insn, file, 1); final (insn, file, 1); final_end_function (); diff --git a/gcc/final.c b/gcc/final.c index 49cfbfb..d2b8523d 100644 --- a/gcc/final.c +++ b/gcc/final.c @@ -1763,9 +1763,11 @@ get_some_local_dynamic_name () test and compare insns. */ void -final_start_function (rtx_insn *first, FILE *file, +final_start_function (rtx_insn **firstp, FILE *file, int optimize_p ATTRIBUTE_UNUSED) { + rtx_insn *first = *firstp; + block_depth = 0; this_is_asm_operands = 0; @@ -4536,8 +4538,9 @@ rest_of_handle_final (void) variable_tracking_main (); assemble_start_function (current_function_decl, fnname); - final_start_function (get_insns (), asm_out_file, optimize); - final (get_insns (), asm_out_file, optimize); + rtx_insn *first = get_insns (); + final_start_function (&first, asm_out_file, optimize); + final (first, asm_out_file, optimize); if (flag_ipa_ra && !lookup_attribute ("noipa", DECL_ATTRIBUTES (current_function_decl))) collect_fn_hard_reg_usage (); diff --git a/gcc/output.h b/gcc/output.h index e98a911..62a405d 100644 --- a/gcc/output.h +++ b/gcc/output.h @@ -59,7 +59,7 @@ const char *get_some_local_dynamic_name (); for the new function. The label for the function and associated assembler pseudo-ops have already been output in `assemble_start_function'. */ -extern void final_start_function (rtx_insn *, FILE *, int); +extern void final_start_function (rtx_insn **, FILE *, int); /* Output assembler code for the end of a function. For clarity, args are same as those of `final_start_function'