Re: Question about patch for PR bootstrap/65150 (identical functions)

Re: Question about patch for PR bootstrap/65150 (identical functions)

[David Edelsohn]

>>> Steve Ellcey wrote:
>
> After your change GCC sees that the code for f1 and f2 are identical
> so it replaced the body of f2 with a call to f1.  This optimization will
> save space but it is not going to be faster because any call to f2 will
> now include an extra call/return.  Do other platforms have this same issue
> or is there a way to make f2 an alias for f1 on other targets so no extra
> call is needed?  I looked around to see if there was a target function or
> macro that is used to make one function an alias of another but I didn't
> see anything.

This probably is not good for POWER, and probably not for ARM, in fact
probably not good for most RISC architectures.

- David

Re: Question about patch for PR bootstrap/65150 (identical functions)

[Jeff Law]

On 05/08/2015 01:07 PM, David Edelsohn wrote:
>>>> Steve Ellcey wrote:
>>
>> After your change GCC sees that the code for f1 and f2 are identical
>> so it replaced the body of f2 with a call to f1.  This optimization will
>> save space but it is not going to be faster because any call to f2 will
>> now include an extra call/return.  Do other platforms have this same issue
>> or is there a way to make f2 an alias for f1 on other targets so no extra
>> call is needed?  I looked around to see if there was a target function or
>> macro that is used to make one function an alias of another but I didn't
>> see anything.
>
> This probably is not good for POWER, and probably not for ARM, in fact
> probably not good for most RISC architectures.
But isn't this code going to be creating aliases when the output format 
supports them -- which avoids the call/return overheads?

Which might argue that if we don't have aliases, then ICF might need to 
be a -Os thing.

jeff

Re: Question about patch for PR bootstrap/65150 (identical functions)

[Jakub Jelinek]

On Fri, May 08, 2015 at 01:09:52PM -0600, Jeff Law wrote:
> On 05/08/2015 01:07 PM, David Edelsohn wrote:
> >>>>Steve Ellcey wrote:
> >>
> >>After your change GCC sees that the code for f1 and f2 are identical
> >>so it replaced the body of f2 with a call to f1.  This optimization will
> >>save space but it is not going to be faster because any call to f2 will
> >>now include an extra call/return.  Do other platforms have this same issue
> >>or is there a way to make f2 an alias for f1 on other targets so no extra
> >>call is needed?  I looked around to see if there was a target function or
> >>macro that is used to make one function an alias of another but I didn't
> >>see anything.
> >
> >This probably is not good for POWER, and probably not for ARM, in fact
> >probably not good for most RISC architectures.
> But isn't this code going to be creating aliases when the output format
> supports them -- which avoids the call/return overheads?
> 
> Which might argue that if we don't have aliases, then ICF might need to be a
> -Os thing.

ICF uses aliases only when possible (when it can prove that the callers
don't care about function address equality/non-equality.  In other cases
ICF uses caller redirection (if it is possible to redirect all callers and
remove unused other call), or thunks (otherwise).

Thunks indeed can slow things down somewhat, especially if they can't tail
call the function.

	Jakub

Re: Question about patch for PR bootstrap/65150 (identical functions)

[Jan Hubicka]

> On Fri, May 08, 2015 at 01:09:52PM -0600, Jeff Law wrote:
> > On 05/08/2015 01:07 PM, David Edelsohn wrote:
> > >>>>Steve Ellcey wrote:
> > >>
> > >>After your change GCC sees that the code for f1 and f2 are identical
> > >>so it replaced the body of f2 with a call to f1.  This optimization will
> > >>save space but it is not going to be faster because any call to f2 will
> > >>now include an extra call/return.  Do other platforms have this same issue
> > >>or is there a way to make f2 an alias for f1 on other targets so no extra
> > >>call is needed?  I looked around to see if there was a target function or
> > >>macro that is used to make one function an alias of another but I didn't
> > >>see anything.
> > >
> > >This probably is not good for POWER, and probably not for ARM, in fact
> > >probably not good for most RISC architectures.
> > But isn't this code going to be creating aliases when the output format
> > supports them -- which avoids the call/return overheads?
> > 
> > Which might argue that if we don't have aliases, then ICF might need to be a
> > -Os thing.
> 
> ICF uses aliases only when possible (when it can prove that the callers
> don't care about function address equality/non-equality.  In other cases
> ICF uses caller redirection (if it is possible to redirect all callers and
> remove unused other call), or thunks (otherwise).
> 
> Thunks indeed can slow things down somewhat, especially if they can't tail
> call the function.

Yep, the idea here is that inliner should undo the splitting in most cases
where the hot path performance matters.
For GCC 6 we plan to work bit on this and try to chose thunks to point in right
direction.  It also may be possible to arrange backend to output those thunks
as simple nops just ahead of the target functions (and also to output thunks
there without the redundant jump instructions)

Honza
> 
> 	Jakub

Re: Question about patch for PR bootstrap/65150 (identical functions)

[Bernhard Reutner-Fischer]

On May 8, 2015 11:58:18 PM GMT+02:00, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Fri, May 08, 2015 at 01:09:52PM -0600, Jeff Law wrote:
>> > On 05/08/2015 01:07 PM, David Edelsohn wrote:
>> > >>>>Steve Ellcey wrote:
>> > >>
>> > >>After your change GCC sees that the code for f1 and f2 are
>identical
>> > >>so it replaced the body of f2 with a call to f1.  This
>optimization will
>> > >>save space but it is not going to be faster because any call to
>f2 will
>> > >>now include an extra call/return.  Do other platforms have this
>same issue
>> > >>or is there a way to make f2 an alias for f1 on other targets so
>no extra
>> > >>call is needed?  I looked around to see if there was a target
>function or
>> > >>macro that is used to make one function an alias of another but I
>didn't
>> > >>see anything.
>> > >
>> > >This probably is not good for POWER, and probably not for ARM, in
>fact
>> > >probably not good for most RISC architectures.
>> > But isn't this code going to be creating aliases when the output
>format
>> > supports them -- which avoids the call/return overheads?
>> > 
>> > Which might argue that if we don't have aliases, then ICF might
>need to be a
>> > -Os thing.
>> 
>> ICF uses aliases only when possible (when it can prove that the
>callers
>> don't care about function address equality/non-equality.  In other
>cases
>> ICF uses caller redirection (if it is possible to redirect all
>callers and
>> remove unused other call), or thunks (otherwise).
>> 
>> Thunks indeed can slow things down somewhat, especially if they can't
>tail
>> call the function.
>
>Yep, the idea here is that inliner should undo the splitting in most
>cases
>where the hot path performance matters.
>For GCC 6 we plan to work bit on this and try to chose thunks to point
>in right
>direction.  It also may be possible to arrange backend to output those
>thunks
>as simple nops just ahead of the target functions (and also to output
>thunks
>there without the redundant jump instructions)

... which will hopefully address https://gcc.gnu.org/PR65534

thanks,

Question about patch for PR bootstrap/65150 (identical functions)

[Steve Ellcey]

Jan and Martin,

I just noticed that your patch for PR bootstrap/65150 broke one of the
MIPS tests (gcc.target/mips/branch-1.c).  I can fix the test with no
problem but I am wondering if the change I am seeing with your patch 
is intended or not.

A cutdown version of the test is:

void bar (void);
void f1 (int x) { if (x & 4) bar (); }
void f2 (int x) { if ((x >> 2) & 1) bar (); }

After your change GCC sees that the code for f1 and f2 are identical
so it replaced the body of f2 with a call to f1.  This optimization will
save space but it is not going to be faster because any call to f2 will
now include an extra call/return.  Do other platforms have this same issue
or is there a way to make f2 an alias for f1 on other targets so no extra
call is needed?  I looked around to see if there was a target function or
macro that is used to make one function an alias of another but I didn't
see anything.

Steve Ellcey
sellcey@imgtec.com