Re: [GOOGLE] More strict checking for call args

[Richard Biener <richard dot guenther at gmail dot com>]

On Wed, Jun 5, 2013 at 6:11 PM, Xinliang David Li <davidxl@google.com> wrote:
> Right, except that in the context of FDO/autoFDO, where this happens
> the most (note in FDO case, it can happen with fresh profile too for
> multi-threaded programs), it is not that important to handle -- the
> mismatch path will never be executed, so why bother to inline and
> bloat the code for it?

It's about being able to IPA-CP through such calls.  Consider

void foo (int);
void bar (int i, float)
{
  foo (i);
}
void foobar ()
{
  bar (1);  // mismatched # of arguments but we can see foo() is
called with constant 1
}

that's important if we can prove that all calls to foo () are called
with a constant 1
argument for example and thus we can replace it without cloning.

If the compatibility predicate guards even the IPA-CP case (not just the
inlining itself) then the predicate is used in a bogus way.

So your symptom is not properly fixed with the patch but papered over.

Richard.

> if (fptr_new == func_old) {
>       func_old (ptr);             <--- do not want to inline.
> }
> else
>    fptr_new(ptr);
>
> David
>
>
> On Wed, Jun 5, 2013 at 1:31 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
>> On Wed, Jun 5, 2013 at 2:19 AM, Dehao Chen <dehao@google.com> wrote:
>>> attached is a testcase that would cause problem when source has changed:
>>>
>>> $ g++ test.cc -O2 -fprofile-generate -DOLD
>>> $ ./a.out
>>> $ g++ test.cc -O2 -fprofile-use
>>> test.cc:34:1: internal compiler error: in operator[], at vec.h:815
>>>  }
>>>  ^
>>> 0x512740 vec<tree_node*, va_heap, vl_embed>::operator[](unsigned int)
>>> ../../gcc/vec.h:815
>>> 0x512740 vec<tree_node*, va_heap, vl_ptr>::operator[](unsigned int)
>>> ../../gcc/vec.h:1244
>>> 0xf24464 vec<tree_node*, va_heap, vl_embed>::operator[](unsigned int)
>>> ../../gcc/vec.h:815
>>> 0xf24464 vec<tree_node*, va_heap, vl_ptr>::operator[](unsigned int)
>>> ../../gcc/vec.h:1244
>>> 0xf24464 ipa_get_indirect_edge_target_1
>>> ../../gcc/ipa-cp.c:1535
>>
>> This use needs to be properly guarded.  We can perfectly well have
>> mismatching fndecl nodes in gimple calls.  If we start with
>>
>> void fn(int, int, int);
>>
>> ...
>> void (*x)(float, double, struct X, int) = fn;
>> (*x)(1., 2., {}, 1);
>>
>> the GIMPLE_CALL receives the function type effective for the call
>> from the source (gimple_call_fntype).  Then CCP happily propagates the
>> 'fn' decl and we end up with
>>
>>   fn (1., 2., {}, 1);
>>
>> that is, gimple_call_fndecl is 'fn' but gimple_call_fntype is still
>> void (*x)(float, double, struct X, int)!
>>
>> So the solution is not to fix the argument verification predicate but to make
>> code aware of the fact that for the call statement gimple_call_fntype is
>> relevant for what is a valid call (that's also what is verified against in
>> verify_stmts) even though the ultimate called function-decl 'fn' has a
>> different prototype.  Thus any code propagating from a call site to
>> the callee has to deal with mismatches.
>>
>> Richard.
>>
>>> 0x971b9a estimate_edge_devirt_benefit
>>> ../../gcc/ipa-inline-analysis.c:2757
>>> 0x973f59 estimate_edge_size_and_time
>>> ../../gcc/ipa-inline-analysis.c:2789
>>> 0x973f59 estimate_calls_size_and_time
>>> ../../gcc/ipa-inline-analysis.c:2842
>>> 0x97429f estimate_node_size_and_time
>>> ../../gcc/ipa-inline-analysis.c:2929
>>> 0x976077 do_estimate_edge_size(cgraph_edge*)
>>> ../../gcc/ipa-inline-analysis.c:3472
>>> 0x97614f estimate_edge_size
>>> ../../gcc/ipa-inline.h:274
>>> 0x97614f estimate_edge_growth
>>> ../../gcc/ipa-inline.h:286
>>> 0x97614f do_estimate_growth_1
>>> ../../gcc/ipa-inline-analysis.c:3582
>>> 0x7e41df cgraph_for_node_and_aliases
>>> ../../gcc/cgraph.c:1777
>>> 0x976675 do_estimate_growth(cgraph_node*)
>>> ../../gcc/ipa-inline-analysis.c:3596
>>> 0xf314ea estimate_growth
>>> ../../gcc/ipa-inline.h:261
>>> 0xf314ea inline_small_functions
>>> ../../gcc/ipa-inline.c:1432
>>> 0xf314ea ipa_inline
>>> ../../gcc/ipa-inline.c:1797
>>> Please submit a full bug report,
>>> with preprocessed source if appropriate.
>>> Please include the complete backtrace with any bug report.
>>> See <http://gcc.gnu.org/bugs.html> for instructions.