qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Aurelien Jarno <aurelien@aurel32.net>
To: Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org, Alexander Graf <agraf@suse.de>
Subject: Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
Date: Mon, 10 Jan 2011 19:37:22 +0100	[thread overview]
Message-ID: <20110110183722.GM17026@hall.aurel32.net> (raw)
In-Reply-To: <4D2A3CD1.2050003@twiddle.net>

On Sun, Jan 09, 2011 at 02:55:13PM -0800, Richard Henderson wrote:
> On 01/09/2011 01:53 PM, Aurelien Jarno wrote:
> >> +    if (inout == val) {
> >> +        TCGType type = rexw ? TCG_TYPE_I64 : TCG_TYPE_I32;
> >> +        TCGRegSet inuse = s->reserved_regs;
> >> +
> >> +        tcg_regset_set_reg(inuse, inout);
> >> +        val = tcg_reg_alloc(s, tcg_target_available_regs[type], inuse);
> >> +
> >> +        tcg_out_mov(s, type, val, inout);
> > 
> > I am a bit worried by allocating a new register here, especially on the
> > i386 target, where the number of free registers is quite low, and often
> > 0. We already had to tweak some code to avoid calls to tcg_abort() due
> > to missing registers.
> 
> Well, as I said, this case can't actually trigger due to a bug in the
> register allocator.  This can be seen in an insn like
> 
> 	mov	%dl,%dh
> 
> where you would expect to see
> 
> 	deposit	x,x,x,8,8
> 
> however, the matching constraint forces the destination and the matching
> source into a new register:
> 
>                 /* if the input is aliased to an output and if it is
>                    not dead after the instruction, we must allocate
>                    a new register and move it */
>                 if (!IS_DEAD_IARG(i - nb_oargs)) 
>                     goto allocate_in_reg;

I have not been able to trigger this code path with a deposit
instruction.

> which means that we'll always see
> 
> 	mov	y,x
> 	deposit y,y,x,8,8
> 
> So I could simply put a tcg_abort there.  It would be up to whoever
> improves the register allocator to provide some mechanism for a
> backend to allocate a scratch.  What do you think?
> 

Do you have a way to trigger this problem? or a dump of the ops and asm
output?

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

  parent reply	other threads:[~2011-01-10 18:37 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-07 22:42 [Qemu-devel] [PATCH 0/7] Define "deposit" tcg operation Richard Henderson
2011-01-07 22:42 ` [Qemu-devel] [PATCH 1/7] tcg: Define "deposit" as an optional operation Richard Henderson
2011-01-07 23:48   ` Stuart Brady
2011-01-09 21:38   ` Aurelien Jarno
2011-01-09 22:45     ` Richard Henderson
2011-01-07 22:42 ` [Qemu-devel] [PATCH 2/7] tcg-ppc: Implement deposit operation Richard Henderson
2011-01-07 23:33   ` [Qemu-devel] " malc
2011-01-07 22:42 ` [Qemu-devel] [PATCH 3/7] tcg-hppa: " Richard Henderson
2011-01-07 23:35   ` Stuart Brady
2011-01-07 23:41     ` Richard Henderson
2011-01-07 22:43 ` [Qemu-devel] [PATCH 4/7] tcg-ia64: " Richard Henderson
2011-01-09 22:04   ` [Qemu-devel] " Aurelien Jarno
2011-01-07 22:43 ` [Qemu-devel] [PATCH 5/7] tcg-i386: " Richard Henderson
2011-01-09 21:53   ` [Qemu-devel] " Aurelien Jarno
2011-01-09 22:55     ` Richard Henderson
2011-01-10  0:16       ` Aurelien Jarno
2011-01-10  0:43         ` Richard Henderson
2011-01-10 16:52           ` Aurelien Jarno
2011-01-10 18:37       ` Aurelien Jarno [this message]
2011-01-10 19:19         ` Richard Henderson
2011-01-07 22:43 ` [Qemu-devel] [PATCH 6/7] target-i386: Use " Richard Henderson
2011-01-07 22:43 ` [Qemu-devel] [PATCH 7/7] target-ppc: " Richard Henderson
2011-01-07 23:10 ` [Qemu-devel] [PATCH 0/7] Define "deposit" tcg operation Peter Maydell
2011-01-07 23:37   ` Richard Henderson
  -- strict thread matches above, loose matches on Subject: below --
2011-01-11  3:23 [Qemu-devel] [PATCH 0/7] Define "deposit" tcg operation, v2 Richard Henderson
2011-01-11  3:23 ` [Qemu-devel] [PATCH 5/7] tcg-i386: Implement deposit operation Richard Henderson
2011-01-25 12:27   ` Edgar E. Iglesias
2011-01-31  8:33     ` Aurelien Jarno
2011-02-08 18:05       ` Richard Henderson
2011-02-09  7:41         ` [Qemu-devel] " Paolo Bonzini
2011-02-09 17:24           ` Blue Swirl

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=20110110183722.GM17026@hall.aurel32.net \
    --to=aurelien@aurel32.net \
    --cc=agraf@suse.de \
    --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).