Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
On Wed, Feb 9, 2011 at 9:41 AM, Paolo Bonzini wrote: > On 02/08/2011 07:05 PM, Richard Henderson wrote: >> >> (Which is probably a mistake from a translator point of view -- there's >> no reason we can't emulate 16-bit operations with 32-bit operations given >> that the high bits are ignorable.) > > Not really, you never know if the guest is going to use a 66 prefix on the > next instruction. Perhaps similar system to current lazy condition code evaluation could be used. The translator would keep record of high bits use status, and if they are getting used, emit extra ops to clear (or recalculate?) the high bits.
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
On 01/10/2011 10:37 AM, Aurelien Jarno wrote: >> 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? IN: 0x408120c4: rlwimi r4,r4,8,16,23 OP: 0x408120c4 deposit_i32 r4,r4,r4,8,8 goto_tb $0x0 movi_i32 nip,$0x408120c8 exit_tb $0x7fbb00ca5758 OUT: [size=52] 0x60294380: mov0x10(%r14),%ebp 0x60294384: mov%ebp,%ebx 0x60294386: ror$0x8,%ebp 0x60294389: shrd $0x8,%ebx,%ebp 0x6029438d: rol$0x10,%ebp 0x60294390: mov%ebp,0x10(%r14) 0x60294394: jmpq 0x60294399 0x60294399: mov$0x408120c8,%ebp 0x6029439e: mov%ebp,0x25c(%r14) 0x602943a5: mov$0x7fbb00ca5758,%rax 0x602943af: jmpq 0x622b772e That should do it. This is present in linux-user-test ppc/ls. This output still contains that allocate-a-scratch path. r~
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
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
[email protected] http://www.aurel32.net
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
On Sun, Jan 09, 2011 at 04:43:22PM -0800, Richard Henderson wrote: > On 01/09/2011 04:16 PM, Aurelien Jarno wrote: > > The code being written now or latter doesn't change the question to know > > if it is always possible to allocate one scratch register here on i386. > > Yes. > > Here there's only one register that needs to remain live. In the > worst case, we'll spill one live register. Which we can always do > via a simple store via AREG0 + offset. Oh right correct we are inside the instruction and in a case we have very few arguments, and only 32-bit. If we allow the target to allocate scratch registers themselves, it's probably better to do that via a function. -- Aurelien Jarno GPG: 1024D/F1BCDB73 [email protected] http://www.aurel32.net
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
On 01/09/2011 04:16 PM, Aurelien Jarno wrote: > The code being written now or latter doesn't change the question to know > if it is always possible to allocate one scratch register here on i386. Yes. Here there's only one register that needs to remain live. In the worst case, we'll spill one live register. Which we can always do via a simple store via AREG0 + offset. r~
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
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 guess we should also add a case when the input argument and the output
argument are the same.
> 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?
>
The code being written now or latter doesn't change the question to know
if it is always possible to allocate one scratch register here on i386.
Up to know we didn't provide any function for allocating a scratch
register, as on CISC CPU the op directly match to an instruction whereas
on RISC CPUs where the op has to be emulated with multiple instructions,
they are enough registers available to define one or more permanent
scratch register.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
[email protected] http://www.aurel32.net
Re: [Qemu-devel] Re: [PATCH 5/7] tcg-i386: Implement deposit operation.
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;
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?
r~
