On Wed, Nov 1, 2017 at 7:18 PM, Alexandre Oliva <aol...@redhat.com> wrote:
> On Oct 31, 2017, Jeff Law <l...@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
