qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: "David Hildenbrand" <david@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Richard W.M. Jones" <rjones@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [PULL 11/24] tcg/optimize: Use tcg_constant_internal with constant folding
Date: Thu, 4 Feb 2021 10:57:32 -0600	[thread overview]
Message-ID: <27958d7a-4908-4ff3-5fd4-c4d34ba18d24@redhat.com> (raw)
In-Reply-To: <74c25c22-eacd-1120-80d4-f721ca9a87ba@redhat.com>

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



  reply	other threads:[~2021-02-04 18:02 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-14  2:16 [PULL 00/24] tcg patch queue Richard Henderson
2021-01-14  2:16 ` [PULL 01/24] tcg: Use tcg_out_dupi_vec from temp_load Richard Henderson
2021-01-14  2:16 ` [PULL 02/24] tcg: Increase tcg_out_dupi_vec immediate to int64_t Richard Henderson
2021-01-14  2:16 ` [PULL 03/24] tcg: Consolidate 3 bits into enum TCGTempKind Richard Henderson
2021-01-14  2:16 ` [PULL 04/24] tcg: Add temp_readonly Richard Henderson
2021-01-14  2:16 ` [PULL 05/24] tcg: Expand TCGTemp.val to 64-bits Richard Henderson
2021-01-14  2:16 ` [PULL 06/24] tcg: Rename struct tcg_temp_info to TempOptInfo Richard Henderson
2021-01-14  2:16 ` [PULL 07/24] tcg: Expand TempOptInfo to 64-bits Richard Henderson
2021-01-14  2:16 ` [PULL 08/24] tcg: Introduce TYPE_CONST temporaries Richard Henderson
2021-01-14  2:16 ` [PULL 09/24] tcg/optimize: Improve find_better_copy Richard Henderson
2021-01-14  2:16 ` [PULL 10/24] tcg/optimize: Adjust TempOptInfo allocation Richard Henderson
2021-01-14  2:16 ` [PULL 11/24] tcg/optimize: Use tcg_constant_internal with constant folding Richard Henderson
2021-01-15 23:03   ` Alistair Francis
2021-01-16 17:24     ` Richard Henderson
2021-01-18 20:17       ` Laurent Vivier
2021-01-18 21:03         ` Richard Henderson
2021-02-01 20:45   ` Richard W.M. Jones
2021-02-01 20:53     ` Richard W.M. Jones
2021-02-04  2:22     ` Richard Henderson
2021-02-04  6:41       ` David Hildenbrand
2021-02-04  7:55         ` David Hildenbrand
2021-02-04  8:38           ` David Hildenbrand
2021-02-04  9:03             ` Richard W.M. Jones
2021-02-04  9:12               ` David Hildenbrand
2021-02-04  9:29               ` Richard W.M. Jones
2021-02-04  9:30                 ` Richard W.M. Jones
2021-02-04  9:37                 ` David Hildenbrand
2021-02-04 16:04                   ` Philippe Mathieu-Daudé
2021-02-04 16:08                     ` Philippe Mathieu-Daudé
2021-02-04 16:12                     ` David Hildenbrand
2021-02-04 16:29                     ` David Hildenbrand
2021-02-04 16:57                       ` Eric Blake [this message]
2021-02-04 17:33                       ` Richard Henderson
2021-02-04 16:48                     ` Richard W.M. Jones
2021-02-04 16:52                     ` Eric Blake
2021-01-14  2:16 ` [PULL 12/24] tcg: Convert tcg_gen_dupi_vec to TCG_CONST Richard Henderson
2021-01-14  2:16 ` [PULL 13/24] tcg: Use tcg_constant_i32 with icount expander Richard Henderson
2021-01-14  2:16 ` [PULL 14/24] tcg: Use tcg_constant_{i32,i64} with tcg int expanders Richard Henderson
2021-02-10 10:38   ` Alex Bennée
2021-01-14  2:16 ` [PULL 15/24] tcg: Use tcg_constant_{i32,i64} with tcg plugins Richard Henderson
2021-01-14  2:16 ` [PULL 16/24] tcg: Use tcg_constant_{i32,i64,vec} with gvec expanders Richard Henderson
2021-01-14  2:16 ` [PULL 17/24] tcg/tci: Add special tci_movi_{i32,i64} opcodes Richard Henderson
2021-01-14  2:16 ` [PULL 18/24] tcg: Remove movi and dupi opcodes Richard Henderson
2021-01-14  2:16 ` [PULL 19/24] tcg: Add tcg_reg_alloc_dup2 Richard Henderson
2021-01-14  2:16 ` [PULL 20/24] tcg/i386: Use tcg_constant_vec with tcg vec expanders Richard Henderson
2021-01-14  2:16 ` [PULL 21/24] tcg: Remove tcg_gen_dup{8,16,32,64}i_vec Richard Henderson
2021-01-14  2:16 ` [PULL 22/24] tcg/ppc: Use tcg_constant_vec with tcg vec expanders Richard Henderson
2021-01-14  2:16 ` [PULL 23/24] tcg/aarch64: " Richard Henderson
2021-01-14  2:16 ` [PULL 24/24] decodetree: Open files with encoding='utf-8' Richard Henderson
2021-01-14 13:13 ` [PULL 00/24] tcg patch queue Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27958d7a-4908-4ff3-5fd4-c4d34ba18d24@redhat.com \
    --to=eblake@redhat.com \
    --cc=david@redhat.com \
    --cc=f4bug@amsat.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).