On Wed, Sep 21, 2016 at 10:45 AM, Andrew Pinski <pins...@gmail.com> wrote:
> On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener
> <richard.guent...@gmail.com> wrote:
>> On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski <pins...@gmail.com> wrote:
>>> Hi,
>>> While looking through bug reports, I noticed someone had reported
>>> that LTO caused the vectorizer not to kick in. Originally it was not
>>> obvious why it would change the behavior since this was a simple
>>> program with nothing out of the ordinary. Well it turned out paths
>>> leading to the exit(0); at the end of main was being marked as cold
>>> and in the LTO case the funciton (which had loop which should have
>>> been vectorized) was considered local and being marked as cold as it
>>> was only called now from the path leading to the exit(0); Since
>>> exit(0); is considered a normal exit path, there is no reason to mark
>>> it as being as a cold path; let the other predicate handle it.
>>>
>>> So this patch changes the predicate for exit(0) not to mark the paths
>>> leading up to that function call as being cold. Note this patch only
>>> disables that when the argument is a literal zero so if we a PHI node
>>> which contains the exit value, we still cause those paths to be
>>> considered cold; this will be for another patch.
>>
>> Hmm, it also doesn't work for main calling a function not returning but
>> exiting
>> with exit (0) (it will be discovered as noreturn).
>>
>> I don't think that treating exit (0) as generally not terminating a cold code
>> sequence is good either.
>>
>> Predictions are always hard I guess ...
>>
>> But the thing to improve is maybe the different handling of main with
>> respect to the guessed profile -- this is something that should not happen
>> inconsistently between LTO / non-LTO as main is special in all cases. Honza?
>
> Maybe I did not explain it very well but what is happening is a
> function which is only called in the cold path is marked as cold only
> in the LTO case as it is extern call.. Basically with LTO, the
> function becomes local so it can be marked as cold while without LTO,
> it cannot.
Ah, so you have
foo () { loop }
main()
{
if ()
{
foo ();
exit (0);
}
...
return 0;
}
and foo is marked cold because its only call is on the path to exit (0)?
noreturn prediction is quite aggressive but it works also quite well. Given
you can only detect a very minor fraction of cases (consider exit (0) being
in foo itself) I'm not sure that your fix is good progress. IMHO we might
want to switch from 'noreturn' to 'noreturn' + likely error which needs
another attribute / auto-discovery and IPA propagation to handle this case
better.
Anyway, I'll leave the patch to Honza.
Richard.
> Thanks,
> Andrew
>
>>
>> Richard.
>>
>>> OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions.
>>> Also tested it with SPEC CPU 2006 with no performance regressions.
>>>
>>> Thanks,
>>> Andrew Pinski
>>>
>>> ChangeLog:
>>> * predict.c (is_exit_with_zero_arg): New function.
>>> (tree_bb_level_predictions): Don't consider paths leading to exit(0)
>>> as nottaken.
