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: address@hidden
>> Signed-off-by: Richard Henderson <address@hidden>
>> ---
>>  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~