qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] tcg/arm: Fix double-word comparisons


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH] tcg/arm: Fix double-word comparisons
Date: Mon, 15 Jan 2018 14:27:00 +0000

On 10 January 2018 at 05:39, Richard Henderson <address@hidden> wrote:
> The code sequence we were generating was only good for unsigned
> comparisons.  For signed comparisions, use the sequence from gcc.
>
> Fixes booting of ppc64 firmware, with a patch changing the code
> sequence for ppc comparisons.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  tcg/arm/tcg-target.inc.c | 112 
> +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 80 insertions(+), 32 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
> index 98a1253..b9890c8 100644
> --- a/tcg/arm/tcg-target.inc.c
> +++ b/tcg/arm/tcg-target.inc.c
> @@ -239,10 +239,11 @@ static void patch_reloc(tcg_insn_unit *code_ptr, int 
> type,
>      }
>  }
>
> -#define TCG_CT_CONST_ARM  0x100
> -#define TCG_CT_CONST_INV  0x200
> -#define TCG_CT_CONST_NEG  0x400
> -#define TCG_CT_CONST_ZERO 0x800
> +#define TCG_CT_CONST_ARM     0x0100
> +#define TCG_CT_CONST_INV     0x0200
> +#define TCG_CT_CONST_NEG     0x0400
> +#define TCG_CT_CONST_INVNEG  0x0800
> +#define TCG_CT_CONST_ZERO    0x1000
>
>  /* parse target specific constraints */
>  static const char *target_parse_constraint(TCGArgConstraint *ct,
> @@ -258,6 +259,9 @@ static const char 
> *target_parse_constraint(TCGArgConstraint *ct,
>      case 'N': /* The gcc constraint letter is L, already used here.  */
>          ct->ct |= TCG_CT_CONST_NEG;
>          break;
> +    case 'M':
> +        ct->ct |= TCG_CT_CONST_INVNEG;
> +        break;

In GCC constraint "M" is "integer in the range 0 to 32", which this clearly
isn't. Can we have a comment saying what it is? (may be worth mentioning
in particular how "rIM" differs from "rINK" -- I think the answer is that
we want to say "valid-immediate OR (valid-if-inverted AND valid-if-negated)",
whereas 'rINK' is "valid-immediate OR valid-if-inverted OR valid-if-negated".)

The code looks OK to me. I'm guessing the sparc guest failure is
just because we now generate more code for some of the comparison
cases and that doesn't fit in the buffer...

We could avoid the annoying "load LE/GE immediates to tempreg"
extra code by having tcg_out_cmp2() return a flag to tell
the caller which way round to put the conditions for its two
conditional ARITH_MOV insns (for setcond2) or which condition
to use for the branch (brcond2), right?

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]