Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Wed, Oct 26, 2016 at 08:38:06AM -0700, Richard Henderson wrote:
> On 10/25/2016 07:59 PM, David Gibson wrote:
> > On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> >> Use the new primitives for RDWINM and RLDICL.
> >>
> >> Cc: qemu-...@nongnu.org
> >> Signed-off-by: Richard Henderson 
> >> ---
> >>  target-ppc/translate.c | 9 -
> >>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> >> index bfc1301..724d95c 100644
> >> --- a/target-ppc/translate.c
> >> +++ b/target-ppc/translate.c
> >> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
> >>  if (mb == 0 && me == (31 - sh)) {
> >>  tcg_gen_shli_tl(t_ra, t_rs, sh);
> >>  tcg_gen_ext32u_tl(t_ra, t_ra);
> >> -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> >> -tcg_gen_ext32u_tl(t_ra, t_rs);
> >> -tcg_gen_shri_tl(t_ra, t_ra, mb);
> >> +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> > 
> > I'm having trouble figuring out what the second part of this condition
> > is supposed to be checking for, and it seems like it's too
> > restrictive.
> > 
> > For example, everything except the LSB of a word would be:
> > rlwnim rT,rA,31,1,31
> > which would fail the test, but it should be fine to implement that
> > with an extract op.
> 
> It was confusing to me too, which is why I rearranged this in the v2 of this
> patchset.  To which thread you also responded yesterday, so...

Ah, sorry.  Because I missed it originally, I only had your ping, not
the actual v2 series in my inbox.  When I went back through my archive
to find it, I accidentally picked up v1 instead of v2.

> 
> Anyway, in v2 this looks like
> 
> if (sh != 0 && len > 0 && me == (31 - sh)) {
> tcg_gen_deposit_z_tl(t_ra, t_rs, sh, len);
> } else if (me == 31 && rsh + len <= 32) {
> tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
> } else {
> 
> Basically, we're trying to match those combinations of rotate+mask that can be
> implemented with shifts instead of real rotations.  That is, the mask doesn't
> follow the rotate around the end of the word.

Ok, that looks correct, the change frm sh to rsh is the fix, I think.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[Richard Henderson]

On 10/25/2016 07:59 PM, David Gibson wrote:
> On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
>> Use the new primitives for RDWINM and RLDICL.
>>
>> Cc: qemu-...@nongnu.org
>> Signed-off-by: Richard Henderson 
>> ---
>>  target-ppc/translate.c | 9 -
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>> index bfc1301..724d95c 100644
>> --- a/target-ppc/translate.c
>> +++ b/target-ppc/translate.c
>> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
>>  if (mb == 0 && me == (31 - sh)) {
>>  tcg_gen_shli_tl(t_ra, t_rs, sh);
>>  tcg_gen_ext32u_tl(t_ra, t_ra);
>> -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
>> -tcg_gen_ext32u_tl(t_ra, t_rs);
>> -tcg_gen_shri_tl(t_ra, t_ra, mb);
>> +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> 
> I'm having trouble figuring out what the second part of this condition
> is supposed to be checking for, and it seems like it's too
> restrictive.
> 
> For example, everything except the LSB of a word would be:
>   rlwnim rT,rA,31,1,31
> which would fail the test, but it should be fine to implement that
> with an extract op.

It was confusing to me too, which is why I rearranged this in the v2 of this
patchset.  To which thread you also responded yesterday, so...

Anyway, in v2 this looks like

if (sh != 0 && len > 0 && me == (31 - sh)) {
tcg_gen_deposit_z_tl(t_ra, t_rs, sh, len);
} else if (me == 31 && rsh + len <= 32) {
tcg_gen_extract_tl(t_ra, t_rs, rsh, len);
} else {

Basically, we're trying to match those combinations of rotate+mask that can be
implemented with shifts instead of real rotations.  That is, the mask doesn't
follow the rotate around the end of the word.


r~

Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> Use the new primitives for RDWINM and RLDICL.
> 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 
> ---
>  target-ppc/translate.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index bfc1301..724d95c 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
>  if (mb == 0 && me == (31 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
>  tcg_gen_ext32u_tl(t_ra, t_ra);
> -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> -tcg_gen_ext32u_tl(t_ra, t_rs);
> -tcg_gen_shri_tl(t_ra, t_ra, mb);
> +} else if (me == 31 && (me - mb + 1) + sh <= 32) {

I'm having trouble figuring out what the second part of this condition
is supposed to be checking for, and it seems like it's too
restrictive.

For example, everything except the LSB of a word would be:
rlwnim rT,rA,31,1,31
which would fail the test, but it should be fine to implement that
with an extract op.

> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  target_ulong mask;
>  #if defined(TARGET_PPC64)
> @@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int 
> me, int sh)
>  
>  if (sh != 0 && mb == 0 && me == (63 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
> -} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
> -tcg_gen_shri_tl(t_ra, t_rs, mb);
> +} else if (me == 63 && (me - mb + 1) + sh <= 64) {
> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  tcg_gen_rotli_tl(t_ra, t_rs, sh);
>  tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Mon, Oct 17, 2016 at 02:38:06PM +1100, David Gibson wrote:
> On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> > Use the new primitives for RDWINM and RLDICL.
> > 
> > Cc: qemu-...@nongnu.org
> > Signed-off-by: Richard Henderson 
> 
> Applied to ppc-for-2.8, thanks.

Uh.. wait.. no, wasn't paying attention to the fact that it needs the
whole series.

> > ---
> >  target-ppc/translate.c | 9 -
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index bfc1301..724d95c 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
> >  if (mb == 0 && me == (31 - sh)) {
> >  tcg_gen_shli_tl(t_ra, t_rs, sh);
> >  tcg_gen_ext32u_tl(t_ra, t_ra);
> > -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> > -tcg_gen_ext32u_tl(t_ra, t_rs);
> > -tcg_gen_shri_tl(t_ra, t_ra, mb);
> > +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> > +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
> >  } else {
> >  target_ulong mask;
> >  #if defined(TARGET_PPC64)
> > @@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int 
> > me, int sh)
> >  
> >  if (sh != 0 && mb == 0 && me == (63 - sh)) {
> >  tcg_gen_shli_tl(t_ra, t_rs, sh);
> > -} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
> > -tcg_gen_shri_tl(t_ra, t_rs, mb);
> > +} else if (me == 63 && (me - mb + 1) + sh <= 64) {
> > +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
> >  } else {
> >  tcg_gen_rotli_tl(t_ra, t_rs, sh);
> >  tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));
> 



-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[David Gibson]

On Sat, Oct 15, 2016 at 08:37:49PM -0700, Richard Henderson wrote:
> Use the new primitives for RDWINM and RLDICL.
> 
> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson 

Applied to ppc-for-2.8, thanks.

> ---
>  target-ppc/translate.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index bfc1301..724d95c 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
>  if (mb == 0 && me == (31 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
>  tcg_gen_ext32u_tl(t_ra, t_ra);
> -} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
> -tcg_gen_ext32u_tl(t_ra, t_rs);
> -tcg_gen_shri_tl(t_ra, t_ra, mb);
> +} else if (me == 31 && (me - mb + 1) + sh <= 32) {
> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  target_ulong mask;
>  #if defined(TARGET_PPC64)
> @@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int 
> me, int sh)
>  
>  if (sh != 0 && mb == 0 && me == (63 - sh)) {
>  tcg_gen_shli_tl(t_ra, t_rs, sh);
> -} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
> -tcg_gen_shri_tl(t_ra, t_rs, mb);
> +} else if (me == 63 && (me - mb + 1) + sh <= 64) {
> +tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
>  } else {
>  tcg_gen_rotli_tl(t_ra, t_rs, sh);
>  tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature