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