qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Laurent Desnogues <laurent.desnogues@gmail.com>
To: malc <av1474@comtv.ru>
Cc: qemu-devel@nongnu.org, Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH 0/7] tcg: conditional set and move opcodes
Date: Thu, 17 Dec 2009 16:37:27 +0100	[thread overview]
Message-ID: <761ea48b0912170737i54f25561q468a4a193114a903@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0912171825330.4454@linmac.oyster.ru>

On Thu, Dec 17, 2009 at 4:32 PM, malc <av1474@comtv.ru> wrote:
[...]
>
> Some:
>  a. It breaks tcg on PPC[1]:
>
>    ...qemu/tcg/tcg.c:1378: tcg fatal error

What a surprise :-)

I can provide a similar patch for ARM (I already have one
for my own implementation of setcond), but I'll wait for this
patch series to stabilize first.


Laurent

>  b. Documentation for movcond has a typo, t0 is assigned not t1
>
>  c. Historically things like that were made conditional with
>    a generic fallback (bswap, neg, not, rot, etc)
>
>  d. Documentation for setcond2 is missing
>
>  e. There's some indentation weirdness here and there and `git am'
>    complains about added trailing whitespace
>
> It would also be interesting to learn what impact adding those two
> has on performance, any results?
>
> [..snip..]
>
> [1] With following i can run some i386 user tests on PPC32 (ls,
>    openssl)
>
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index 07e6941..195af13 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -316,6 +316,7 @@ static int tcg_target_const_match(tcg_target_long val,
>  #define STH    OPCD(44)
>  #define STW    OPCD(36)
>
> +#define ADDIC  OPCD(12)
>  #define ADDI   OPCD(14)
>  #define ADDIS  OPCD(15)
>  #define ORI    OPCD(24)
> @@ -339,6 +340,7 @@ static int tcg_target_const_match(tcg_target_long val,
>  #define CRANDC XO19(129)
>  #define CRNAND XO19(225)
>  #define CROR   XO19(449)
> +#define CRNOR  XO19( 33)
>
>  #define EXTSB  XO31(954)
>  #define EXTSH  XO31(922)
> @@ -365,6 +367,8 @@ static int tcg_target_const_match(tcg_target_long val,
>  #define MTSPR  XO31(467)
>  #define SRAWI  XO31(824)
>  #define NEG    XO31(104)
> +#define MFCR   XO31( 19)
> +#define CNTLZW XO31( 26)
>
>  #define LBZX   XO31( 87)
>  #define LHZX   XO31(279)
> @@ -1073,6 +1077,95 @@ static void tcg_out_brcond (TCGContext *s, int cond,
>     tcg_out_bc (s, tcg_to_bc[cond], label_index);
>  }
>
> +static void tcg_out_setcond (TCGContext *s, int cond, TCGArg arg0,
> +                             TCGArg arg1, TCGArg arg2, int const_arg2)
> +{
> +    int crop, sh;
> +
> +    switch (cond) {
> +    case TCG_COND_EQ:
> +        if (const_arg2) {
> +            if ((uint16_t) arg2 == arg2) {
> +                tcg_out32 (s, XORI | RS (arg1) | RA (0) | arg2);
> +            }
> +            else {
> +                tcg_out_movi (s, TCG_TYPE_I32, 0, arg2);
> +                tcg_out32 (s, XOR | SAB (arg1, 0, 0));
> +            }
> +        }
> +        else {
> +            tcg_out32 (s, XOR | SAB (arg1, 0, arg2));
> +        }
> +        tcg_out32 (s, CNTLZW | RS (0) | RA (0));
> +        tcg_out32 (s, (RLWINM
> +                       | RA (arg0)
> +                       | RS (0)
> +                       | SH (27)
> +                       | MB (5)
> +                       | ME (31)
> +                       )
> +            );
> +        return;
> +
> +    case TCG_COND_NE:
> +        if (const_arg2) {
> +            if ((uint16_t) arg2 == arg2) {
> +                tcg_out32 (s, XORI | RS (arg1) | RA (0) | arg2);
> +            }
> +            else {
> +                tcg_out_movi (s, TCG_TYPE_I32, 0, arg2);
> +                tcg_out32 (s, XOR | SAB (arg1, 0, 0));
> +            }
> +        }
> +        else {
> +            tcg_out32 (s, XOR | SAB (arg1, 0, arg2));
> +        }
> +
> +        tcg_out32 (s, ADDIC | RT (arg0) | RA (0) | 0xffff);
> +        tcg_out32 (s, SUBFE | TAB (arg0, arg0, 0));
> +        return;
> +
> +    case TCG_COND_LTU:
> +    case TCG_COND_LT:
> +        sh = 29;
> +        crop = 0;
> +        break;
> +
> +    case TCG_COND_GEU:
> +    case TCG_COND_GE:
> +        sh = 31;
> +        crop = CRNOR | BT (7, CR_EQ) | BA (7, CR_LT) | BB (7, CR_LT);
> +        break;
> +
> +    case TCG_COND_LEU:
> +    case TCG_COND_LE:
> +        sh = 31;
> +        crop = CRNOR | BT (7, CR_EQ) | BA (7, CR_GT) | BB (7, CR_GT);
> +        break;
> +
> +    case TCG_COND_GTU:
> +    case TCG_COND_GT:
> +        sh = 30;
> +        crop = 0;
> +        break;
> +
> +    default:
> +        tcg_abort ();
> +    }
> +
> +    tcg_out_cmp (s, cond, arg1, arg2, const_arg2, 7);
> +    tcg_out32 (s, MFCR | RT (0));
> +    if (crop) tcg_out32 (s, crop);
> +    tcg_out32 (s, (RLWINM
> +                   | RA (arg0)
> +                   | RS (0)
> +                   | SH (sh)
> +                   | MB (31)
> +                   | ME (31)
> +                   )
> +            );
> +}
> +
>  /* XXX: we implement it at the target level to avoid having to
>    handle cross basic blocks temporaries */
>  static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args,
> @@ -1496,6 +1589,10 @@ static void tcg_out_op(TCGContext *s, int opc, const TCGArg *args,
>         tcg_out32 (s, EXTSH | RS (args[1]) | RA (args[0]));
>         break;
>
> +    case INDEX_op_setcond_i32:
> +        tcg_out_setcond(s, args[3], args[0], args[1], args[2], const_args[2]);
> +        break;
> +
>     default:
>         tcg_dump_ops (s, stderr);
>         tcg_abort ();
> @@ -1544,6 +1641,8 @@ static const TCGTargetOpDef ppc_op_defs[] = {
>
>     { INDEX_op_neg_i32, { "r", "r" } },
>
> +    { INDEX_op_setcond_i32, { "r", "r", "ri" } },
> +
>  #if TARGET_LONG_BITS == 32
>     { INDEX_op_qemu_ld8u, { "r", "L" } },
>     { INDEX_op_qemu_ld8s, { "r", "L" } },
>
> --
> mailto:av1474@comtv.ru
>
>
>

  reply	other threads:[~2009-12-17 15:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-17  1:19 [Qemu-devel] [PATCH 0/7] tcg: conditional set and move opcodes Richard Henderson
2009-12-16  0:34 ` [Qemu-devel] [PATCH 1/7] tcg: Generic support for conditional set and conditional move Richard Henderson
2009-12-16  0:35 ` [Qemu-devel] [PATCH 2/7] tcg-amd64: Implement setcond and movcond Richard Henderson
2009-12-16  0:36 ` [Qemu-devel] [PATCH 3/7] target-alpha: Use setcond/movcond in integer compares and cmoves Richard Henderson
2009-12-16 23:17 ` [Qemu-devel] [PATCH 4/7] tcg-i386: Implement setcond, movcond, setcond2 Richard Henderson
2009-12-16 23:26 ` [Qemu-devel] [PATCH 5/7] tcg-sparc: Implement setcond, movcond, setcond2, brcond2 Richard Henderson
2009-12-19 10:31   ` Blue Swirl
2009-12-19 17:47     ` Richard Henderson
2009-12-19 21:25       ` Blue Swirl
2009-12-19 22:52         ` Richard Henderson
2009-12-20 11:06           ` Blue Swirl
2009-12-16 23:28 ` [Qemu-devel] [PATCH 6/7] target-i386: Use setcond and movcond Richard Henderson
2009-12-16 23:29 ` [Qemu-devel] [PATCH 7/7] target-mips: " Richard Henderson
2009-12-17 15:32 ` [Qemu-devel] [PATCH 0/7] tcg: conditional set and move opcodes malc
2009-12-17 15:37   ` Laurent Desnogues [this message]
2009-12-17 17:07   ` Richard Henderson
2009-12-17 17:47     ` malc
2009-12-17 18:09       ` Richard Henderson
2009-12-17 17:48     ` Richard Henderson
2009-12-18 15:40     ` malc
2009-12-18 16:05       ` Richard Henderson

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=761ea48b0912170737i54f25561q468a4a193114a903@mail.gmail.com \
    --to=laurent.desnogues@gmail.com \
    --cc=av1474@comtv.ru \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /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).