This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] Add a lexical block only when the callsite has source location info
- From: Richard Guenther <richard dot guenther at gmail dot com>
- To: Dehao Chen <dehao at google dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Cary Coutant <ccoutant at google dot com>
- Date: Mon, 25 Jun 2012 14:02:13 +0200
- Subject: Re: [patch] Add a lexical block only when the callsite has source location info
- References: <CAO2gOZUGYop71A25c33wUNbjmJ9idzp7iy5Vk6C5hAD0v1ZONw@mail.gmail.com>
On Mon, Jun 25, 2012 at 1:43 PM, Dehao Chen <dehao@google.com> wrote:
> During function inlining, a lexical block is added for each cloned
> callee, and source info is attached to this block for addr2line to
> derive the inline stack.
Well - the bug is then clearly
/* Set input_location here so we get the right instantiation context
if we call instantiate_decl from inlinable_function_p. */
saved_location = input_location;
if (gimple_has_location (stmt))
input_location = gimple_location (stmt)
which retails input_location instead of setting it to UNKNOWN_LOCATION.
Not adding a BLOCK will make debug information incorrect, no?
> However, some callsites do not have source
> information attached to it. Adding a lexical block would be misleading
> in this case. E.g. If a function is split, when the split callsite is
> inlined back, the cloned callee should stay in the same lexical block
> with its caller. This patch ensures that lexical blocks are only added
> when the callsite has source location info in it.
>
> Bootstrapped and passed gcc regression tests.
>
> Is it ok for trunk?
I'd rather see an unconditional set of input_location from gimple_location
of the statement.
Richard.
> Thanks,
> Dehao
>
> gcc/ChangeLog:
> 2012-06-25 ?Dehao Chen ?<dehao@google.com>
>
> ? ? ? ?* tree-profile.c: (expand_call_inline): Make a new lexical block only
^^^^^
tree-inline.c
> ? ? ? ?when the call stmt has source location.
>
> Index: gcc/tree-inline.c
> ===================================================================
> --- gcc/tree-inline.c ? (revision 188926)
> +++ gcc/tree-inline.c ? (working copy)
> @@ -3950,10 +3950,17 @@
> ? ? ?actual inline expansion of the body, and a label for the return
> ? ? ?statements within the function to jump to. ?The type of the
> ? ? ?statement expression is the return type of the function call. ?*/
> - ?id->block = make_node (BLOCK);
> - ?BLOCK_ABSTRACT_ORIGIN (id->block) = fn;
> - ?BLOCK_SOURCE_LOCATION (id->block) = input_location;
> - ?prepend_lexical_block (gimple_block (stmt), id->block);
> + ?if (gimple_has_location (stmt))
> + ? ?{
> + ? ? ?id->block = make_node (BLOCK);
> + ? ? ?BLOCK_ABSTRACT_ORIGIN (id->block) = fn;
> + ? ? ?BLOCK_SOURCE_LOCATION (id->block) = input_location;
Please use gimple_location (stmt) instead of input_location (yes, I realize
its set from that).
> + ? ? ?prepend_lexical_block (gimple_block (stmt), id->block);
> + ? ?}
> + ?else
> + ? ?{
> + ? ? ?id->block = gimple_block (stmt);
> + ? ?}
> ? /* Local declarations will be replaced by their equivalents in this
> ? ? ?map. ?*/