[tree-inline] do not say "called from here" with UNKNOWN_LOCATION
diff mbox

Message ID CAESRpQDA02cNVYCA+T7cu9u6isD1FwO3OEYZ1KadqYMw8z6DMA@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Sept. 18, 2015, 6:47 p.m. UTC
And now with the patch.

On 18 September 2015 at 20:40, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> In https://sourceware.org/ml/libc-alpha/2014-12/msg00300.html, we give a
> "called from here" note without actually having a location, which looks
> strange. I haven't been able to generate such a testcase. In this patch, we
> assert this cannot happen when checking and simply skip the extra note in
> release mode.
>
> Boot&tested on x86_64-linux-gnu
>
> OK?
>
>
> gcc/testsuite/ChangeLog:
>
> 2015-09-18  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     * gcc.target/i386/inline_error.c (int bar): Use dg-message for note.
>     * gcc.target/i386/pr57756.c (static __inline int caller): Likewise.
>     * gcc.target/i386/pr59789.c (f1): Likewise.
>     * gcc.target/i386/intrinsics_5.c (__m128i foo): Likewise.
>     * gcc.target/i386/intrinsics_6.c: Likewise.
>     * gcc.dg/winline-5.c (int t): Likewise.
>     * gcc.dg/winline-9.c (t): Likewise.
>     * gcc.dg/always_inline2.c (q): Likewise.
>     * gcc.dg/winline-2.c (inline int t): Likewise.
>     * gcc.dg/winline-6.c: Likewise.
>     * gcc.dg/winline-10.c (void g): Likewise.
>     * gcc.dg/pr49243.c (void parse): Likewise.
>     * gcc.dg/always_inline3.c (q2): Likewise.
>     * gcc.dg/winline-3.c: Likewise.
>     * gcc.dg/winline-7.c (inline void *t): Likewise.
>
> gcc/ChangeLog:
>
> 2015-09-18  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>
>     * tree-inline.c (expand_call_inline): Use inform for extra note.
>     Do not give "called from here" with UNKNOWN_LOCATION.

Comments

Richard Biener Sept. 21, 2015, 8:18 a.m. UTC | #1
On Fri, Sep 18, 2015 at 8:47 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> And now with the patch.
>
> On 18 September 2015 at 20:40, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> In https://sourceware.org/ml/libc-alpha/2014-12/msg00300.html, we give a
>> "called from here" note without actually having a location, which looks
>> strange. I haven't been able to generate such a testcase. In this patch, we
>> assert this cannot happen when checking and simply skip the extra note in
>> release mode.
>>
>> Boot&tested on x86_64-linux-gnu
>>
>> OK?

Hmm.

input_location is set from the call stmt:

  /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
  saved_location = input_location;
  input_location = gimple_location (stmt);

it would be nice to get rid of that.

If the call is artificially generated it might have no location so I
think the assert
is bogus.  Also in the case of UNKNOWN_LOCATION it would be nice to at
least note the function we are failing to inline to (thus, use
DECL_SOURCE_LOCATION
of cfun->decl).  So better add a diag_location and compute that upfront to avoid
repeating the check.

Did you investigate the glibc case on whether caller or callee are artificial?

Richard.

>>
>> gcc/testsuite/ChangeLog:
>>
>> 2015-09-18  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>>
>>     * gcc.target/i386/inline_error.c (int bar): Use dg-message for note.
>>     * gcc.target/i386/pr57756.c (static __inline int caller): Likewise.
>>     * gcc.target/i386/pr59789.c (f1): Likewise.
>>     * gcc.target/i386/intrinsics_5.c (__m128i foo): Likewise.
>>     * gcc.target/i386/intrinsics_6.c: Likewise.
>>     * gcc.dg/winline-5.c (int t): Likewise.
>>     * gcc.dg/winline-9.c (t): Likewise.
>>     * gcc.dg/always_inline2.c (q): Likewise.
>>     * gcc.dg/winline-2.c (inline int t): Likewise.
>>     * gcc.dg/winline-6.c: Likewise.
>>     * gcc.dg/winline-10.c (void g): Likewise.
>>     * gcc.dg/pr49243.c (void parse): Likewise.
>>     * gcc.dg/always_inline3.c (q2): Likewise.
>>     * gcc.dg/winline-3.c: Likewise.
>>     * gcc.dg/winline-7.c (inline void *t): Likewise.
>>
>> gcc/ChangeLog:
>>
>> 2015-09-18  Manuel López-Ibáñez  <manu@gcc.gnu.org>
>>
>>     * tree-inline.c (expand_call_inline): Use inform for extra note.
>>     Do not give "called from here" with UNKNOWN_LOCATION.
Manuel López-Ibáñez Sept. 21, 2015, 9:59 a.m. UTC | #2
On 21 September 2015 at 10:18, Richard Biener
<richard.guenther@gmail.com> wrote:
> input_location is set from the call stmt:
>
>   /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
>   saved_location = input_location;
>   input_location = gimple_location (stmt);
>
> it would be nice to get rid of that.

I could replace all uses of input_location in this function by
gimple_location(stmt) as I noted in the comments. Would that be ok if
it works? I'm not sure I can prove that input_location is not used
behind the scenes for some other purpose (all the more reason to kill
input_location once and for all). Friends, don't let friends use
input_location in new code!

> If the call is artificially generated it might have no location so I
> think the assert
> is bogus.  Also in the case of UNKNOWN_LOCATION it would be nice to at

I was sure of that, but I was also surprised that not a single
testcase was triggering it. I can re-submit without the assert.

> least note the function we are failing to inline to (thus, use
> DECL_SOURCE_LOCATION
> of cfun->decl).  So better add a diag_location and compute that upfront to avoid
> repeating the check.

       error ("inlining failed in call to always_inline %q+F: %s", fn,
          cgraph_inline_failed_string (reason));

The call is using '+F', thus the location is set to some location
related to F, depending on which *_printer function is active at that
moment. cp_printer uses location_of, and default_tree_printer uses
DECL_SOURCE_LOCATION. Is the default_tree_printer always used at this
point? If yes, I completely agree we should use an explicit
DECL_SOURCE_LOCATION. The meaning of '+' is not only opaque but it
breaks #pragma GCC diagnostic.

> Did you investigate the glibc case on whether caller or callee are artificial?

No, I could not get glibc to compile. Unfortunately, the machine of
the compile farm that I use to work on GCC is too old. Installing
locally all up-to-date dependencies would require too much time from
me that is better spent on other things. Nevertheless, printing "cc1:
called from here" is always bogus. Not printing it is better. If we
could print something smarter, that would be perfect, but I don't know
how to do that, so "better" is good enough for me. Would you agree?

Cheers,

Manuel.
Richard Biener Sept. 21, 2015, 10:29 a.m. UTC | #3
On Mon, Sep 21, 2015 at 11:59 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 21 September 2015 at 10:18, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> input_location is set from the call stmt:
>>
>>   /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
>>   saved_location = input_location;
>>   input_location = gimple_location (stmt);
>>
>> it would be nice to get rid of that.
>
> I could replace all uses of input_location in this function by
> gimple_location(stmt) as I noted in the comments. Would that be ok if
> it works? I'm not sure I can prove that input_location is not used
> behind the scenes for some other purpose (all the more reason to kill
> input_location once and for all). Friends, don't let friends use
> input_location in new code!

Yeah...  not sure how to check but to look for any changes in
generated cc1/cc1plus
debug info.  You could also try making it invalid (-1?) and hope
libcpp would eventually
blow up if that is used.

>> If the call is artificially generated it might have no location so I
>> think the assert
>> is bogus.  Also in the case of UNKNOWN_LOCATION it would be nice to at
>
> I was sure of that, but I was also surprised that not a single
> testcase was triggering it. I can re-submit without the assert.

please

>> least note the function we are failing to inline to (thus, use
>> DECL_SOURCE_LOCATION
>> of cfun->decl).  So better add a diag_location and compute that upfront to avoid
>> repeating the check.
>
>        error ("inlining failed in call to always_inline %q+F: %s", fn,
>           cgraph_inline_failed_string (reason));
>
> The call is using '+F', thus the location is set to some location
> related to F, depending on which *_printer function is active at that
> moment. cp_printer uses location_of, and default_tree_printer uses
> DECL_SOURCE_LOCATION. Is the default_tree_printer always used at this
> point? If yes, I completely agree we should use an explicit
> DECL_SOURCE_LOCATION. The meaning of '+' is not only opaque but it
> breaks #pragma GCC diagnostic.

But it prints the location of the function we failed to inline.  I
want to retain
at least an approximation to the location of the call, which is the location
of the function we inline _to_.

>
>> Did you investigate the glibc case on whether caller or callee are artificial?
>
> No, I could not get glibc to compile. Unfortunately, the machine of
> the compile farm that I use to work on GCC is too old. Installing
> locally all up-to-date dependencies would require too much time from
> me that is better spent on other things. Nevertheless, printing "cc1:
> called from here" is always bogus. Not printing it is better. If we
> could print something smarter, that would be perfect, but I don't know
> how to do that, so "better" is good enough for me. Would you agree?

Well, it's easy to do better, see above.

Richard.

> Cheers,
>
> Manuel.
Manuel López-Ibáñez Sept. 21, 2015, 10:46 a.m. UTC | #4
On 21 September 2015 at 12:29, Richard Biener
<richard.guenther@gmail.com> wrote:
>>> least note the function we are failing to inline to (thus, use
>>> DECL_SOURCE_LOCATION
>>> of cfun->decl).  So better add a diag_location and compute that upfront to avoid
>>> repeating the check.
>>
>>        error ("inlining failed in call to always_inline %q+F: %s", fn,
>>           cgraph_inline_failed_string (reason));
>>
>> The call is using '+F', thus the location is set to some location
>> related to F, depending on which *_printer function is active at that
>> moment. cp_printer uses location_of, and default_tree_printer uses
>> DECL_SOURCE_LOCATION. Is the default_tree_printer always used at this
>> point? If yes, I completely agree we should use an explicit
>> DECL_SOURCE_LOCATION. The meaning of '+' is not only opaque but it
>> breaks #pragma GCC diagnostic.
>
> But it prints the location of the function we failed to inline.  I
> want to retain
> at least an approximation to the location of the call, which is the location
> of the function we inline _to_.

I think I misunderstood you. Do you mean something like?

if (gimple_location (stmt) != UNKNOWN_LOCATION)
        inform (gimple_location (stmt), "called from here");
else
        inform (DECL_SOURCE_LOCATION (cfun->decl), "called from this function");


Cheers,

Manuel.
Richard Biener Sept. 21, 2015, 10:57 a.m. UTC | #5
On Mon, Sep 21, 2015 at 12:46 PM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 21 September 2015 at 12:29, Richard Biener
> <richard.guenther@gmail.com> wrote:
>>>> least note the function we are failing to inline to (thus, use
>>>> DECL_SOURCE_LOCATION
>>>> of cfun->decl).  So better add a diag_location and compute that upfront to avoid
>>>> repeating the check.
>>>
>>>        error ("inlining failed in call to always_inline %q+F: %s", fn,
>>>           cgraph_inline_failed_string (reason));
>>>
>>> The call is using '+F', thus the location is set to some location
>>> related to F, depending on which *_printer function is active at that
>>> moment. cp_printer uses location_of, and default_tree_printer uses
>>> DECL_SOURCE_LOCATION. Is the default_tree_printer always used at this
>>> point? If yes, I completely agree we should use an explicit
>>> DECL_SOURCE_LOCATION. The meaning of '+' is not only opaque but it
>>> breaks #pragma GCC diagnostic.
>>
>> But it prints the location of the function we failed to inline.  I
>> want to retain
>> at least an approximation to the location of the call, which is the location
>> of the function we inline _to_.
>
> I think I misunderstood you. Do you mean something like?
>
> if (gimple_location (stmt) != UNKNOWN_LOCATION)
>         inform (gimple_location (stmt), "called from here");
> else
>         inform (DECL_SOURCE_LOCATION (cfun->decl), "called from this function");

Yes.  (now that location may also be UNKNOWN in which case I don't
have a good fallback idea and we can drop the note)

Richard.

>
> Cheers,
>
> Manuel.
Trevor Saunders Sept. 22, 2015, 12:18 a.m. UTC | #6
On Mon, Sep 21, 2015 at 12:29:36PM +0200, Richard Biener wrote:
> On Mon, Sep 21, 2015 at 11:59 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
> > On 21 September 2015 at 10:18, Richard Biener
> > <richard.guenther@gmail.com> wrote:
> >> input_location is set from the call stmt:
> >>
> >>   /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
> >>   saved_location = input_location;
> >>   input_location = gimple_location (stmt);
> >>
> >> it would be nice to get rid of that.
> >
> > I could replace all uses of input_location in this function by
> > gimple_location(stmt) as I noted in the comments. Would that be ok if
> > it works? I'm not sure I can prove that input_location is not used
> > behind the scenes for some other purpose (all the more reason to kill
> > input_location once and for all). Friends, don't let friends use
> > input_location in new code!
> 
> Yeah...  not sure how to check but to look for any changes in
> generated cc1/cc1plus
> debug info.  You could also try making it invalid (-1?) and hope
> libcpp would eventually
> blow up if that is used.

I've been kind of meaning to add some raii classes like
auto_assert_no_input_location that assert if you use input_location
while its on the stack.  It shouldn't be that hard to do, so it'd be
cool if someone beat me to getting it done :)

Trev
Manuel López-Ibáñez Sept. 22, 2015, 12:22 a.m. UTC | #7
On 21 September 2015 at 12:29, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Mon, Sep 21, 2015 at 11:59 AM, Manuel López-Ibáñez
> <lopezibanez@gmail.com> wrote:
>> On 21 September 2015 at 10:18, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>> input_location is set from the call stmt:
>>>
>>>   /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
>>>   saved_location = input_location;
>>>   input_location = gimple_location (stmt);
>>>
>>> it would be nice to get rid of that.
>>
>> I could replace all uses of input_location in this function by
>> gimple_location(stmt) as I noted in the comments. Would that be ok if
>> it works? I'm not sure I can prove that input_location is not used
>> behind the scenes for some other purpose (all the more reason to kill
>> input_location once and for all). Friends, don't let friends use
>> input_location in new code!
>
> Yeah...  not sure how to check but to look for any changes in
> generated cc1/cc1plus
> debug info.  You could also try making it invalid (-1?) and hope
> libcpp would eventually
> blow up if that is used.
>

It does blow up with:

/home/manuel/test2/227965M/build/gcc/gnat1 -gnatwa -quiet -nostdinc
-dumpbase s-mudido.adb -auxbase-strip s-mudido.o -O2 -Wextra -Wall
-fpic -g -gnatpg -mtune=generic -march=x86-64 -gnatO s-mudido.o
s-mudido.adb -o /tmp/ccNQpzNF.s

at:

B =>SET_EXPR_LOCATION (mod, EXPR_LOC_OR_LOC (val, input_location));

#0  internal_get_tmp_var (val=0x7ffff5dfdc40, pre_p=0x7fffffffdaf0,
post_p=<optimized out>, is_formal=<optimized out>) at
/home/manuel/test2/src/gcc/gimplify.c:540
#1  0x0000000000c00efd in gimplify_expr
(expr_p=expr_p@entry=0x7fffffffdaf8, pre_p=pre_p@entry=0x7fffffffdaf0,
post_p=0x7fffffffd9a0, post_p@entry=0x0, gimple_test_f=<optimized
out>, fallback=fallback@entry=1) at
/home/manuel/test2/src/gcc/gimplify.c:9040
#2  0x0000000000c19b67 in gimple_regimplify_operands
(stmt=0x7ffff6074be0, gsi_p=gsi_p@entry=0x7fffffffdbb0) at
/home/manuel/test2/src/gcc/gimplify-me.c:252
#3  0x0000000000e8cbe3 in copy_bb (id=id@entry=0x7fffffffde40,
bb=bb@entry=0x7ffff60eb548,
frequency_scale=frequency_scale@entry=10000,
count_scale=count_scale@entry=10000) at
/home/manuel/test2/src/gcc/tree-inline.c:1798
#4  0x0000000000e8e039 in copy_cfg_body (new_entry=0x0,
exit_block_map=0x7ffff5dff340, entry_block_map=0x7ffff60d4c30,
frequency_scale=10000, count=<optimized out>, id=0x7fffffffde40) at
/home/manuel/test2/src/gcc/tree-inline.c:2716
#5  copy_body (id=0x7fffffffde40, count=<optimized out>,
frequency_scale=10000, entry_block_map=0x7ffff60d4c30,
exit_block_map=0x7ffff5dff340, new_entry=0x0) at
/home/manuel/test2/src/gcc/tree-inline.c:2955
#6  0x0000000000e94f71 in expand_call_inline (id=0x7fffffffde40,
stmt=<optimized out>, bb=<optimized out>) at
/home/manuel/test2/src/gcc/tree-inline.c:4693
#7  gimple_expand_calls_inline (id=0x7fffffffde40, bb=<optimized out>)
at /home/manuel/test2/src/gcc/tree-inline.c:4833
#8  optimize_inline_calls (fn=<optimized out>) at
/home/manuel/test2/src/gcc/tree-inline.c:4973
#9  0x00000000014c503c in inline_transform (node=0x7ffff644ccf0) at
/home/manuel/test2/src/gcc/ipa-inline-transform.c:545
#10 0x0000000000d54bac in execute_one_ipa_transform_pass
(ipa_pass=0x2656340, node=0x7ffff644ccf0) at
/home/manuel/test2/src/gcc/passes.c:2197
#11 execute_all_ipa_transforms () at /home/manuel/test2/src/gcc/passes.c:2238
#12 0x0000000000a99bc8 in cgraph_node::expand
(this=this@entry=0x7ffff644ccf0) at
/home/manuel/test2/src/gcc/cgraphunit.c:1976
#13 0x0000000000a9b44e in expand_all_functions () at
/home/manuel/test2/src/gcc/cgraphunit.c:2119
#14 symbol_table::compile (this=this@entry=0x7ffff642b0a8) at
/home/manuel/test2/src/gcc/cgraphunit.c:2472
#15 0x0000000000a9da63 in symbol_table::compile (this=0x7ffff642b0a8)
at /home/manuel/test2/src/gcc/cgraphunit.c:2536
#16 symbol_table::finalize_compilation_unit (this=0x7ffff642b0a8) at
/home/manuel/test2/src/gcc/cgraphunit.c:2562
#17 0x0000000000e17d90 in compile_file () at
/home/manuel/test2/src/gcc/toplev.c:508
#18 0x000000000069e8a4 in do_compile () at
/home/manuel/test2/src/gcc/toplev.c:1973
#19 toplev::main (this=this@entry=0x7fffffffe0a0, argc=argc@entry=21,
argv=argv@entry=0x7fffffffe198) at
/home/manuel/test2/src/gcc/toplev.c:2080
#20 0x00000000006a0bd7 in main (argc=21, argv=0x7fffffffe198) at
/home/manuel/test2/src/gcc/main.c:39

For some extra reason val does not have a location:

(gdb) p debug_tree(val)
 <addr_expr 0x7ffff5dfdc40
    type <pointer_type 0x7ffff5fca540
        type <array_type 0x7ffff5fca2a0
system__multiprocessors__dispatching_domains__create__Tst_ddS__2 type
<boolean_type 0x7ffff6445dc8 boolean>
            sizes-gimplified asm_written nonaliased-component BLK size
<var_decl 0x7ffff606b5a0 iftmp.56> unit size <var_decl 0x7ffff606b870
iftmp.57>
            align 8 symtab -166627696 alias set 32 canonical type
0x7ffff5fca2a0 domain <integer_type 0x7ffff5fca1f8> context
<function_decl 0x7ffff5fbc460 system__multiprocessors__\
dispatching_domains__create__2>
            pointer_to_this <pointer_type 0x7ffff5fca540>
reference_to_this <reference_type 0x7ffff5fdc1f8> chain <type_decl
0x7ffff5fbfc78 system__multiprocessors__dispatching_doma\
ins__create__Tst_ddS__2>>
        asm_written public unsigned DI
        size <integer_cst 0x7ffff6427bb8 constant 64>
        unit size <integer_cst 0x7ffff6427bd0 constant 8>
        align 64 symtab -166626736 alias set 41 canonical type 0x7ffff5fca540>

    arg 0 <mem_ref 0x7ffff5dfecd0 type <array_type 0x7ffff5fca2a0
system__multiprocessors__dispatching_domains__create__Tst_ddS__2>
        nothrow
        arg 0 <ssa_name 0x7ffff5d52dc8 type <pointer_type 0x7ffff5fca540>
            visited var <var_decl 0x7ffff607ca20 R.94>def_stmt
R.94_159 = .builtin_alloca_with_align (iftmp.93_10, 8);

            version 159
            ptr-info 0x7ffff5d7d5a0>
        arg 1 <integer_cst 0x7ffff6084030 constant 0>
        s-mudido.adb:156:24>>

Arguably, gimple_location (stmt) is probably better than anything that
may be initially at input_location.

Then, we cannot remove the saved_location hack, right?

Cheers,

Manuel.
Richard Biener Sept. 22, 2015, 8:21 a.m. UTC | #8
On Tue, Sep 22, 2015 at 2:22 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 21 September 2015 at 12:29, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Mon, Sep 21, 2015 at 11:59 AM, Manuel López-Ibáñez
>> <lopezibanez@gmail.com> wrote:
>>> On 21 September 2015 at 10:18, Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>> input_location is set from the call stmt:
>>>>
>>>>   /* FIXME: instantiate_decl isn't called by inlinable_function_p.  */
>>>>   saved_location = input_location;
>>>>   input_location = gimple_location (stmt);
>>>>
>>>> it would be nice to get rid of that.
>>>
>>> I could replace all uses of input_location in this function by
>>> gimple_location(stmt) as I noted in the comments. Would that be ok if
>>> it works? I'm not sure I can prove that input_location is not used
>>> behind the scenes for some other purpose (all the more reason to kill
>>> input_location once and for all). Friends, don't let friends use
>>> input_location in new code!
>>
>> Yeah...  not sure how to check but to look for any changes in
>> generated cc1/cc1plus
>> debug info.  You could also try making it invalid (-1?) and hope
>> libcpp would eventually
>> blow up if that is used.
>>
>
> It does blow up with:
>
> /home/manuel/test2/227965M/build/gcc/gnat1 -gnatwa -quiet -nostdinc
> -dumpbase s-mudido.adb -auxbase-strip s-mudido.o -O2 -Wextra -Wall
> -fpic -g -gnatpg -mtune=generic -march=x86-64 -gnatO s-mudido.o
> s-mudido.adb -o /tmp/ccNQpzNF.s
>
> at:
>
> B =>SET_EXPR_LOCATION (mod, EXPR_LOC_OR_LOC (val, input_location));
>
> #0  internal_get_tmp_var (val=0x7ffff5dfdc40, pre_p=0x7fffffffdaf0,
> post_p=<optimized out>, is_formal=<optimized out>) at
> /home/manuel/test2/src/gcc/gimplify.c:540
> #1  0x0000000000c00efd in gimplify_expr
> (expr_p=expr_p@entry=0x7fffffffdaf8, pre_p=pre_p@entry=0x7fffffffdaf0,
> post_p=0x7fffffffd9a0, post_p@entry=0x0, gimple_test_f=<optimized
> out>, fallback=fallback@entry=1) at
> /home/manuel/test2/src/gcc/gimplify.c:9040
> #2  0x0000000000c19b67 in gimple_regimplify_operands
> (stmt=0x7ffff6074be0, gsi_p=gsi_p@entry=0x7fffffffdbb0) at
> /home/manuel/test2/src/gcc/gimplify-me.c:252
> #3  0x0000000000e8cbe3 in copy_bb (id=id@entry=0x7fffffffde40,
> bb=bb@entry=0x7ffff60eb548,
> frequency_scale=frequency_scale@entry=10000,
> count_scale=count_scale@entry=10000) at
> /home/manuel/test2/src/gcc/tree-inline.c:1798
> #4  0x0000000000e8e039 in copy_cfg_body (new_entry=0x0,
> exit_block_map=0x7ffff5dff340, entry_block_map=0x7ffff60d4c30,
> frequency_scale=10000, count=<optimized out>, id=0x7fffffffde40) at
> /home/manuel/test2/src/gcc/tree-inline.c:2716
> #5  copy_body (id=0x7fffffffde40, count=<optimized out>,
> frequency_scale=10000, entry_block_map=0x7ffff60d4c30,
> exit_block_map=0x7ffff5dff340, new_entry=0x0) at
> /home/manuel/test2/src/gcc/tree-inline.c:2955
> #6  0x0000000000e94f71 in expand_call_inline (id=0x7fffffffde40,
> stmt=<optimized out>, bb=<optimized out>) at
> /home/manuel/test2/src/gcc/tree-inline.c:4693
> #7  gimple_expand_calls_inline (id=0x7fffffffde40, bb=<optimized out>)
> at /home/manuel/test2/src/gcc/tree-inline.c:4833
> #8  optimize_inline_calls (fn=<optimized out>) at
> /home/manuel/test2/src/gcc/tree-inline.c:4973
> #9  0x00000000014c503c in inline_transform (node=0x7ffff644ccf0) at
> /home/manuel/test2/src/gcc/ipa-inline-transform.c:545
> #10 0x0000000000d54bac in execute_one_ipa_transform_pass
> (ipa_pass=0x2656340, node=0x7ffff644ccf0) at
> /home/manuel/test2/src/gcc/passes.c:2197
> #11 execute_all_ipa_transforms () at /home/manuel/test2/src/gcc/passes.c:2238
> #12 0x0000000000a99bc8 in cgraph_node::expand
> (this=this@entry=0x7ffff644ccf0) at
> /home/manuel/test2/src/gcc/cgraphunit.c:1976
> #13 0x0000000000a9b44e in expand_all_functions () at
> /home/manuel/test2/src/gcc/cgraphunit.c:2119
> #14 symbol_table::compile (this=this@entry=0x7ffff642b0a8) at
> /home/manuel/test2/src/gcc/cgraphunit.c:2472
> #15 0x0000000000a9da63 in symbol_table::compile (this=0x7ffff642b0a8)
> at /home/manuel/test2/src/gcc/cgraphunit.c:2536
> #16 symbol_table::finalize_compilation_unit (this=0x7ffff642b0a8) at
> /home/manuel/test2/src/gcc/cgraphunit.c:2562
> #17 0x0000000000e17d90 in compile_file () at
> /home/manuel/test2/src/gcc/toplev.c:508
> #18 0x000000000069e8a4 in do_compile () at
> /home/manuel/test2/src/gcc/toplev.c:1973
> #19 toplev::main (this=this@entry=0x7fffffffe0a0, argc=argc@entry=21,
> argv=argv@entry=0x7fffffffe198) at
> /home/manuel/test2/src/gcc/toplev.c:2080
> #20 0x00000000006a0bd7 in main (argc=21, argv=0x7fffffffe198) at
> /home/manuel/test2/src/gcc/main.c:39
>
> For some extra reason val does not have a location:
>
> (gdb) p debug_tree(val)
>  <addr_expr 0x7ffff5dfdc40
>     type <pointer_type 0x7ffff5fca540
>         type <array_type 0x7ffff5fca2a0
> system__multiprocessors__dispatching_domains__create__Tst_ddS__2 type
> <boolean_type 0x7ffff6445dc8 boolean>
>             sizes-gimplified asm_written nonaliased-component BLK size
> <var_decl 0x7ffff606b5a0 iftmp.56> unit size <var_decl 0x7ffff606b870
> iftmp.57>
>             align 8 symtab -166627696 alias set 32 canonical type
> 0x7ffff5fca2a0 domain <integer_type 0x7ffff5fca1f8> context
> <function_decl 0x7ffff5fbc460 system__multiprocessors__\
> dispatching_domains__create__2>
>             pointer_to_this <pointer_type 0x7ffff5fca540>
> reference_to_this <reference_type 0x7ffff5fdc1f8> chain <type_decl
> 0x7ffff5fbfc78 system__multiprocessors__dispatching_doma\
> ins__create__Tst_ddS__2>>
>         asm_written public unsigned DI
>         size <integer_cst 0x7ffff6427bb8 constant 64>
>         unit size <integer_cst 0x7ffff6427bd0 constant 8>
>         align 64 symtab -166626736 alias set 41 canonical type 0x7ffff5fca540>
>
>     arg 0 <mem_ref 0x7ffff5dfecd0 type <array_type 0x7ffff5fca2a0
> system__multiprocessors__dispatching_domains__create__Tst_ddS__2>
>         nothrow
>         arg 0 <ssa_name 0x7ffff5d52dc8 type <pointer_type 0x7ffff5fca540>
>             visited var <var_decl 0x7ffff607ca20 R.94>def_stmt
> R.94_159 = .builtin_alloca_with_align (iftmp.93_10, 8);
>
>             version 159
>             ptr-info 0x7ffff5d7d5a0>
>         arg 1 <integer_cst 0x7ffff6084030 constant 0>
>         s-mudido.adb:156:24>>
>
> Arguably, gimple_location (stmt) is probably better than anything that
> may be initially at input_location.
>
> Then, we cannot remove the saved_location hack, right?

Not without fixing that function, no.  I don't see why that should need to play
with locations at all though (or why using input_location -- IIRC gimplification
also sets that, so that's probably where it "leaks" from)

Richard.

> Cheers,
>
> Manuel.

Patch
diff mbox

Index: gcc/testsuite/gcc.target/i386/inline_error.c
===================================================================
--- gcc/testsuite/gcc.target/i386/inline_error.c	(revision 227880)
+++ gcc/testsuite/gcc.target/i386/inline_error.c	(working copy)
@@ -7,7 +7,7 @@  foo () /* { dg-error "inlining failed in
   return 0;
 }
 
 int bar()
 {
-  return foo (); /* { dg-error "called from here" } */
+  return foo (); /* { dg-message "called from here" } */
 }
Index: gcc/testsuite/gcc.target/i386/pr57756.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr57756.c	(revision 227880)
+++ gcc/testsuite/gcc.target/i386/pr57756.c	(working copy)
@@ -9,11 +9,11 @@  __inline int callee () /* { dg-error "in
 }
 
 __attribute__((target("sse")))
 static __inline int caller ()
 {
-  return callee(); /* { dg-error "called from here" }  */
+  return callee(); /* { dg-message "called from here" }  */
 }
 
 int main ()
 {
   return caller();
Index: gcc/testsuite/gcc.target/i386/pr59789.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr59789.c	(revision 227880)
+++ gcc/testsuite/gcc.target/i386/pr59789.c	(working copy)
@@ -16,7 +16,7 @@  _mm_set_epi32 (int __q3, int __q2, int _
 
 
 __m128i
 f1(void)
 { /* { dg-message "warning: SSE vector return without SSE enabled changes the ABI" } */
-  return _mm_set_epi32 (0, 0, 0, 0); /* { dg-error "called from here" } */
+  return _mm_set_epi32 (0, 0, 0, 0); /* { dg-message "called from here" } */
 }
Index: gcc/testsuite/gcc.target/i386/intrinsics_5.c
===================================================================
--- gcc/testsuite/gcc.target/i386/intrinsics_5.c	(revision 227880)
+++ gcc/testsuite/gcc.target/i386/intrinsics_5.c	(working copy)
@@ -8,9 +8,9 @@ 
 
 #include <smmintrin.h>
 
 __m128i foo(__m128i *V)
 {
-    return _mm_stream_load_si128(V); /* { dg-error "called from here" } */
+    return _mm_stream_load_si128(V); /* { dg-message "called from here" } */
 }
 
 /* { dg-prune-output ".*inlining failed.*" }  */
Index: gcc/testsuite/gcc.target/i386/intrinsics_6.c
===================================================================
--- gcc/testsuite/gcc.target/i386/intrinsics_6.c	(revision 227880)
+++ gcc/testsuite/gcc.target/i386/intrinsics_6.c	(working copy)
@@ -8,9 +8,9 @@ 
 
 #include <smmintrin.h>
 
 __m128i foo(__m128i *V)
 {
-    return _mm_stream_load_si128(V); /* { dg-error "called from here" } */
+    return _mm_stream_load_si128(V); /* { dg-message "called from here" } */
 }
 
 /* { dg-prune-output ".*inlining failed.*" }  */
Index: gcc/testsuite/gcc.dg/winline-5.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-5.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-5.c	(working copy)
@@ -15,7 +15,7 @@  inline int q(void) /* { dg-warning "inli
 	big();
 	big();
 }
 int t (void)
 {
-	return q ();		 /* { dg-warning "called from here" } */
+	return q ();		 /* { dg-message "called from here" } */
 }
Index: gcc/testsuite/gcc.dg/winline-9.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-9.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-9.c	(working copy)
@@ -20,7 +20,7 @@  int
 t()
 {
   if (a)
     aa();
   if (b)
-    bb(); 			/* { dg-warning "called from here" "" } */
+    bb(); 			/* { dg-message "called from here" "" } */
 }
Index: gcc/testsuite/gcc.dg/always_inline2.c
===================================================================
--- gcc/testsuite/gcc.dg/always_inline2.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/always_inline2.c	(working copy)
@@ -2,7 +2,7 @@ 
 /* { dg-options "-O2 -fgnu89-inline" } */
 inline __attribute__ ((always_inline)) void t(void); /* { dg-error "body not available" } */
 void
 q(void)
 {
-  t(); 				/* { dg-error "called from here" } */
+  t(); 				/* { dg-message "called from here" } */
 }
Index: gcc/testsuite/gcc.dg/winline-2.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-2.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-2.c	(working copy)
@@ -2,7 +2,7 @@ 
 /* { dg-options "-Winline -O2 -fgnu89-inline" } */
 
 inline int q(void);		 /* { dg-warning "body not available" "" } */
 inline int t(void)
 {
-	return q();		 /* { dg-warning "called from here" "" } */
+	return q();		 /* { dg-message "called from here" "" } */
 }
Index: gcc/testsuite/gcc.dg/winline-6.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-6.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-6.c	(working copy)
@@ -15,7 +15,7 @@  inline int q(void) /* { dg-warning "larg
 	big();
 	big();
 }
 inline int t (void)
 {
-	return q () + 1;	 /* { dg-warning "called from here" } */
+	return q () + 1;	 /* { dg-message "called from here" } */
 }
Index: gcc/testsuite/gcc.dg/winline-10.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-10.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-10.c	(working copy)
@@ -9,9 +9,9 @@  inline void f (x)	/* { dg-warning "inlin
   asm ("");
 }
 
 void g (struct s x)
 {
-  f (x); 		/* { dg-warning "called from here" "" } */
+  f (x); 		/* { dg-message "called from here" "" } */
 }
 
 void f (int x);		/* { dg-warning "follows non-prototype definition" } */
Index: gcc/testsuite/gcc.dg/pr49243.c
===================================================================
--- gcc/testsuite/gcc.dg/pr49243.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/pr49243.c	(working copy)
@@ -18,8 +18,8 @@  static inline int wrapper(const char **s
 }
 
 void parse(const char *data)
 {
     const char *s = data;
-    if (!(wrapper(&s) == -1 && (s - data) == 1)) /* { dg-warning "called from here" } */
+    if (!(wrapper(&s) == -1 && (s - data) == 1)) /* { dg-message "called from here" } */
 	__builtin_abort();
 }
Index: gcc/testsuite/gcc.dg/always_inline3.c
===================================================================
--- gcc/testsuite/gcc.dg/always_inline3.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/always_inline3.c	(working copy)
@@ -4,8 +4,8 @@  int do_something_evil (void);
 inline __attribute__ ((always_inline)) void
 q2(void) /* { dg-error "recursive inlining" } */
 {
   if (do_something_evil ())
     return;
-  q2(); 			/* { dg-error "called from here" } */
+  q2(); 			/* { dg-message "called from here" } */
   q2(); /* With -O2 we don't warn here, it is eliminated by tail recursion.  */
 }
Index: gcc/testsuite/gcc.dg/winline-3.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-3.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-3.c	(working copy)
@@ -15,7 +15,7 @@  inline int q(void) /* { dg-warning "max-
 	big();
 	big();
 }
 inline int t (void)
 {
-	return q ();		 /* { dg-warning "called from here" "" } */
+	return q ();		 /* { dg-message "called from here" "" } */
 }
Index: gcc/testsuite/gcc.dg/winline-7.c
===================================================================
--- gcc/testsuite/gcc.dg/winline-7.c	(revision 227880)
+++ gcc/testsuite/gcc.dg/winline-7.c	(working copy)
@@ -9,7 +9,7 @@  inline void *q (void) /* { dg-warning "(
 {
 	return alloca (10);
 }
 inline void *t (void)
 {
-	return q ();		 /* { dg-warning "called from here" } */
+	return q ();		 /* { dg-message "called from here" } */
 }
Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c	(revision 227880)
+++ gcc/tree-inline.c	(working copy)
@@ -4452,11 +4452,14 @@  expand_call_inline (basic_block bb, gimp
 	  /* PR 20090218-1_0.c. Body can be provided by another module. */
 	  && (reason != CIF_BODY_NOT_AVAILABLE || !flag_generate_lto))
 	{
 	  error ("inlining failed in call to always_inline %q+F: %s", fn,
 		 cgraph_inline_failed_string (reason));
-	  error ("called from here");
+	  if (input_location != UNKNOWN_LOCATION)
+	    /* ??? Use gimple_location (stmt) instead ? */
+	    inform (input_location, "called from here");
+	  gcc_checking_assert (input_location != UNKNOWN_LOCATION);
 	}
       else if (warn_inline
 	       && DECL_DECLARED_INLINE_P (fn)
 	       && !DECL_NO_INLINE_WARNING_P (fn)
 	       && !DECL_IN_SYSTEM_HEADER (fn)
@@ -4465,13 +4468,19 @@  expand_call_inline (basic_block bb, gimp
 	       /* Do not warn about not inlined recursive calls.  */
 	       && !cg_edge->recursive_p ()
 	       /* Avoid warnings during early inline pass. */
 	       && symtab->global_info_ready)
 	{
-	  warning (OPT_Winline, "inlining failed in call to %q+F: %s",
-		   fn, _(cgraph_inline_failed_string (reason)));
-	  warning (OPT_Winline, "called from here");
+	  /* FIXME: input_location should never be UNKNOWN_LOCATION
+	     here, but it seems it can happen:
+	     https://sourceware.org/ml/libc-alpha/2014-12/msg00300.html  */
+	  if (warning (OPT_Winline, "inlining failed in call to %q+F: %s",
+		       fn, _(cgraph_inline_failed_string (reason)))
+	      && input_location != UNKNOWN_LOCATION)
+	    /* ??? Use gimple_location (stmt) instead ? */
+	    inform (input_location, "called from here");
+	  gcc_checking_assert (input_location != UNKNOWN_LOCATION);
 	}
       goto egress;
     }
   fn = cg_edge->callee->decl;
   cg_edge->callee->get_untransformed_body ();
@@ -4532,10 +4541,11 @@  expand_call_inline (basic_block bb, gimp
      not refer to them in any way to not break GC for locations.  */
   if (gimple_block (stmt))
     {
       id->block = make_node (BLOCK);
       BLOCK_ABSTRACT_ORIGIN (id->block) = fn;
+      /* FIXME: gimple_location (stmt) ?  */
       BLOCK_SOURCE_LOCATION (id->block) = LOCATION_LOCUS (input_location);
       prepend_lexical_block (gimple_block (stmt), id->block);
     }
 
   /* Local declarations will be replaced by their equivalents in this