Re: [PULL 11/24] tcg/optimize: Use tcg_constant_internal with constant folding

[Eric Blake]

On 2/4/21 10:29 AM, David Hildenbrand wrote:

>> +++ b/tcg/s390/tcg-target.c.inc
>> @@ -1094,10 +1094,16 @@ static int tgen_cmp(TCGContext *s, TCGType type,
>> TCGCond c, TCGReg r1,
>>                   op = (is_unsigned ? RIL_CLFI : RIL_CFI);
>>                   tcg_out_insn_RIL(s, op, r1, c2);
>>                   goto exit;
>> -            } else if (c2 == (is_unsigned ? (uint32_t)c2 :
>> (int32_t)c2)) {
>> -                op = (is_unsigned ? RIL_CLGFI : RIL_CGFI);
>> -                tcg_out_insn_RIL(s, op, r1, c2);
>> -                goto exit;
>> +            } else if (is_unsigned) {
>> +                if (c2 == (uint32_t)c2) {
>> +                    tcg_out_insn_RIL(s, RIL_CLGFI, r1, c2);
>> +                    goto exit;
>> +                }
>> +            } else {
>> +                if (c2 == (int32_t)c2) {
>> +                    tcg_out_insn_RIL(s, RIL_CGFI, r1, c2);
>> +                    goto exit;
>> +                }
>>               }
>>           }
>> ---
>>
> 
> This works as well I think. Broken casting.
> 
> diff --git a/tcg/s390/tcg-target.c.inc b/tcg/s390/tcg-target.c.inc
> index 8517e55232..f41ca02492 100644
> --- a/tcg/s390/tcg-target.c.inc
> +++ b/tcg/s390/tcg-target.c.inc
> @@ -1094,7 +1094,7 @@ static int tgen_cmp(TCGContext *s, TCGType type,
> TCGCond c, TCGReg r1,
>                  op = (is_unsigned ? RIL_CLFI : RIL_CFI);
>                  tcg_out_insn_RIL(s, op, r1, c2);
>                  goto exit;
> -            } else if (c2 == (is_unsigned ? (uint32_t)c2 : (int32_t)c2)) {
> +            } else if (c2 == (is_unsigned ? (TCGArg)(uint32_t)c2 :
> (TCGArg)(int32_t)c2)) {

Yes, that also solves your problem in fewer lines of code, by doing the
round-trip parsing through the intermediate type and back to the desired
common type all at one expression, rather than separated at two
different points where intermediate type promotion rules interfere with
the work.

>                  op = (is_unsigned ? RIL_CLGFI : RIL_CGFI);
> 
> (int32_t)c2 will be converted to (uint32_t) first, then to (TCGArg).
> Which is different than casting directly from (int32_t)c2 to (TCGArg).

Yep, the broken version was losing out on the desired sign extensions
because of the argument promotion rules of ?:

> 
> Nasty.
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org