Re: rs6000 and ffs

[Andrew Pinski <pinskia at physics dot uc dot edu>]

Here is the updated patch with your comments taken into account.

On Sunday, Dec 1, 2002, at 18:12 US/Pacific, David Edelsohn wrote:

> 2002-12-01 Andrew Pinski <pinskia@physics.uc.edu>
>
> 	* config/rs6000/rs6000.md: Document unspec ID
> 	for cntlz{d,w}2. (ffssi): Change to an expand.
> 	(ffsdi): Likewise. (cntlzw2): Add.
> 	(cntlzd2): Likewise.
>
> This concept is fine.
>
> 	If you are going to convert the two ffs patterns into expanders,
> why are you only generating one psuedo for the intermediate operations?
This was my first real looking into the machine description file, that 
is why I generated only one psuedo.

> You should create a pseudo for each step and let the compiler collapse 
> the
> pseudos during register allocation so that CSE can have opportunities 
> to
> re-use intermediate computations.
I see a case where one intermediate computation is used but it should 
have reused the whole computation (temp),
but if you add in that you want another part of that intermediate 
computation it will reuse it (temp1):
long temp(long i, long *c)
{
        *c = ffs(-i);
        return ffs(i);
}

long temp1(long i, long *c, long *d)
{
        *d = (-i)&(i);
        *c = ffs(-i);
        return ffs(i);
}

> 	Why did you add a commutative modifier to the output constraint?
>
> (set (match_operand:SI 0 "gpc_reg_operand" "=%r")
> 					    -^-
>
> With what is the output operand suppose to commute?
I was finding what the '%' or even '&' meant in the docs until you said 
anything.
I just removed them because they were wrong.

Ok for b-i-b?

Thanks,
Andrew Pinski
pinskia@physics.uc.edu
apinski@apple.com

Attachment: temp.diff
Description: Binary data