Re: [PATCH] (PR 65950) Improve predicate for exit(0);
> > 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)?
>
>
> Actually the case I have here is just:
> foo () { loop }
> int main(void)
> {
> .
> foo();
> ...
> exit (0);
> }
>
> Just an exit at the end of main.
> Basically if we treat exit(0) as a normal function, main becomes hot again.
Yep, it is old noreturn predicate lazynes. Path is OK
Honza
>
> Thanks,
> Andrew
>
> >
> > 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.
Re: [PATCH] (PR 65950) Improve predicate for exit(0);
On Wed, Sep 21, 2016 at 1:53 AM, Richard Biener
wrote:
> On Wed, Sep 21, 2016 at 10:45 AM, Andrew Pinski wrote:
>> On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener
>> wrote:
>>> On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski 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)?
Actually the case I have here is just:
foo () { loop }
int main(void)
{
.
foo();
...
exit (0);
}
Just an exit at the end of main.
Basically if we treat exit(0) as a normal function, main becomes hot again.
Thanks,
Andrew
>
> 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.
Re: [PATCH] (PR 65950) Improve predicate for exit(0);
On Wed, Sep 21, 2016 at 10:45 AM, Andrew Pinski wrote:
> On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener
> wrote:
>> On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski 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.
Re: [PATCH] (PR 65950) Improve predicate for exit(0);
On Wed, Sep 21, 2016 at 4:32 PM, Richard Biener wrote: > On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski 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. 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.
Re: [PATCH] (PR 65950) Improve predicate for exit(0);
On Wed, Sep 21, 2016 at 9:13 AM, Andrew Pinski 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? 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.
