Re: [Qemu-devel] [PATCH v1] target-s390x: fix risbg handling

[David Hildenbrand]

On 01.07.2017 22:27, Richard Henderson wrote:
> On 06/25/2017 03:19 PM, Aurelien Jarno wrote:
>> On 2017-06-23 01:12, David Hildenbrand wrote:
>>> If we have for example: r3 contains 0x00000000ffffffff
>>>      ec 33 3f bf 61 55       risbg   %r3,%r3,63,191,97
>>>
>>> We want to rotate 33 to the left and only keep MSB bit 63 of that. So the
>>> result is then exactly 1 (we're reading the sign of the 32 bit value).
>>>
>>> Current code assumes that we can do that via an extract, which is not
>>> true (at least not that easy) and produces a 0.
>>
>> I think the mistake there is that the rotation is done to the left,
>> while in extract the "shift" is done to the right. The following patch
>> should be enough:
>>
>> --- a/target/s390x/translate.c
>> +++ b/target/s390x/translate.c
>> @@ -3441,8 +3441,8 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps 
>> *o)
>>       }
>>   
>>       /* In some cases we can implement this with extract.  */
>> -    if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) {
>> -        tcg_gen_extract_i64(o->out, o->in2, rot, len);
>> +    if (imask == 0 && pos == 0 && len > 0 && rot - len >= 0) {
>> +        tcg_gen_extract_i64(o->out, o->in2, 64 - rot, len);
>>           return NO_EXIT;
> 
> Agreed.  Included.
> 
> 
> r~
> 

Was able to test it with your version and it works just fine!

Thanks!

-- 

Thanks,

David