qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <rth@twiddle.net>
To: jiang.biao2@zte.com.cn
Cc: qemu-devel@nongnu.org, james.hogan@imgtec.com,
	zhong.weidong@zte.com.cn, wang.liang82@zte.com.cn,
	shi.zhongbing@zte.com.cn, jinguojie@loongson.cn,
	jiang.yong5@zte.com.cn
Subject: Re: [Qemu-devel] [PATCH] tcg/mips: Bugfix for crash when runningprogram with qemu-i386.
Date: Sun, 9 Jul 2017 16:15:57 -1000	[thread overview]
Message-ID: <584340a3-950c-293b-c874-c5eb10686bb8@twiddle.net> (raw)
In-Reply-To: <201707101004393739204@zte.com.cn>

On 07/09/2017 04:04 PM, jiang.biao2@zte.com.cn wrote:
>  >>       if (TCG_TARGET_REG_BITS > TARGET_LONG_BITS) {
>  >>           tcg_out_ext32u(s, base, addr_regl);
>  >> -        addr_regl = base;
>  >> +        tcg_out_mov(s, TCG_TYPE_PTR, addr_regl, base);
>  >>       }
>  >>       if (guest_base == 0 && data_regl != addr_regl) {
>  >>           base = addr_regl;
>  >
>  > This is wrong, because you're not allowed to modify the input operands.
>  >
>  > Try this, just a few lines lower in the function:
> 
>  > -        tcg_out_movi(s, TCG_TYPE_PTR, base, guest_base);
>  > -        tcg_out_opc_reg(s, ALIAS_PADD, base, base, addr_regl);
>  > +        tcg_out_movi(s, TCG_TYPE_PTR, TCG_TMP0, guest_base);
>  > +        tcg_out_opc_reg(s, ALIAS_PADD, base, TCG_TMP0, addr_regl);
> 
>  >
> 
> Got it, but the real problem is for addr_regl instead of guest_base.

Guest base is a problem simply because we require a temporary for it, and we 
were trying to put two temporaries into the same register.

If we retain guest_base in a register all of the time, then (1) we do not have 
to recompute it for every memory load and (2) we do not need a temporary for it.

>  > Better would be to reserve a register for the guest_base, like we do for ppc.
>  > See all of the uses of TCG_GUEST_BASE_REG in tcg/ppc/tcg-target.inc.c.
> 
> It uses base(TCG_REG_A0) for temperary use for guest_base in this case.

No it doesn't.  It computes guest_base into a register in the prologue:

> #ifndef CONFIG_SOFTMMU
>     if (guest_base) {
>         tcg_out_movi(s, TCG_TYPE_PTR, TCG_GUEST_BASE_REG, guest_base);
>         tcg_regset_set_reg(s->reserved_regs, TCG_GUEST_BASE_REG);
>     }
> #endif

and then it uses a reg+reg addressing mode during qemu_ld/st:

>     rbase = guest_base ? TCG_GUEST_BASE_REG : 0;
...
>             insn = qemu_ldx_opc[opc & (MO_SIZE | MO_BSWAP)];
>             tcg_out32(s, insn | TAB(datalo, rbase, addrlo));

Obviously mips doesn't have a reg+reg addressing mode, so a PADD instruction is 
required, but otherwise you can use the same scheme.  Using TCG_REG_S1 on mips 
for TCG_GUEST_BASE_REG would be fine.


r~

      reply	other threads:[~2017-07-10  2:17 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-10  2:04 [Qemu-devel] [PATCH] tcg/mips: Bugfix for crash when runningprogram with qemu-i386 jiang.biao2
2017-07-10  2:15 ` Richard Henderson [this message]

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=584340a3-950c-293b-c874-c5eb10686bb8@twiddle.net \
    --to=rth@twiddle.net \
    --cc=james.hogan@imgtec.com \
    --cc=jiang.biao2@zte.com.cn \
    --cc=jiang.yong5@zte.com.cn \
    --cc=jinguojie@loongson.cn \
    --cc=qemu-devel@nongnu.org \
    --cc=shi.zhongbing@zte.com.cn \
    --cc=wang.liang82@zte.com.cn \
    --cc=zhong.weidong@zte.com.cn \
    /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).