qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC v1 16/23] riscv: tcg-target: Add slowpath load and


From: Alistair Francis
Subject: Re: [Qemu-devel] [RFC v1 16/23] riscv: tcg-target: Add slowpath load and store instructions
Date: Tue, 20 Nov 2018 16:18:12 -0800

On Fri, Nov 16, 2018 at 1:24 AM Richard Henderson
<address@hidden> wrote:
>
> On 11/15/18 11:36 PM, Alistair Francis wrote:
> > +static void tcg_out_mb(TCGContext *s, TCGArg a0)
> > +{
> > +    static const RISCVInsn fence[] = {
> > +        [0 ... TCG_MO_ALL] = OPC_FENCE_RW_RW,
> > +        [TCG_MO_LD_LD]     = OPC_FENCE_R_R,
> > +        [TCG_MO_ST_LD]     = OPC_FENCE_W_R,
> > +        [TCG_MO_LD_ST]     = OPC_FENCE_R_W,
> > +        [TCG_MO_ST_ST]     = OPC_FENCE_W_W,
> > +        [TCG_BAR_LDAQ]     = OPC_FENCE_R_RW,
> > +        [TCG_BAR_STRL]     = OPC_FENCE_RW_W,
> > +        [TCG_BAR_SC]       = OPC_FENCE_RW_RW,
> > +    };
> > +    tcg_out32(s, fence[a0 & TCG_MO_ALL]);
> > +}
> > +
>
> TCG_MO_* and TCG_BAR_* are two different bitmasks, or'ed together.
> Which you've separated by "& TCG_MO_ALL".  Thus the TCG_BAR_* constants should
> not appear in this table.
>
>
> > +static void * const qemu_ld_helpers[16] = {
> > +    [MO_UB]   = helper_ret_ldub_mmu,
> > +    [MO_SB]   = helper_ret_ldsb_mmu,
> > +    [MO_LEUW] = helper_le_lduw_mmu,
> > +    [MO_LESW] = helper_le_ldsw_mmu,
> > +    [MO_LEUL] = helper_le_ldul_mmu,
> > +    [MO_LESL] = helper_le_ldsl_mmu,
> > +    [MO_LEQ]  = helper_le_ldq_mmu,
> > +    [MO_BEUW] = helper_be_lduw_mmu,
> > +    [MO_BESW] = helper_be_ldsw_mmu,
> > +    [MO_BEUL] = helper_be_ldul_mmu,
> > +    [MO_BESL] = helper_be_ldsl_mmu,
> > +    [MO_BEQ]  = helper_be_ldq_mmu,
> > +};
>
> The LESL and BESL functions will not be present for rv32 -> link error.  Here
> you do need an ifdef.
>
> > +        } else {
> > +            adj = cmp_off - sextract32(cmp_off, 0, 12);
> > +            tcg_debug_assert(add_off - adj >= -0x1000
> > +                             && add_off - adj < 0x1000);
> > +
> > +            tcg_out_opc_upper(s, OPC_LUI, base, adj);
> > +            tcg_out_opc_reg(s, OPC_ADD, base, TCG_REG_ZERO, TCG_AREG0);
>
> base, base, TCG_AREG0.
>
> > +    /* Compare masked address with the TLB entry. */
> > +    label_ptr[0] = s->code_ptr;
> > +    tcg_out_opc_branch(s, OPC_BNE, TCG_REG_TMP0, TCG_REG_TMP1, 0);
>
> Another case of a potential out-of-range branch.
>
> It might be worthwhile to move all of this out-of-line from the start, where
> that branch will always be short.  See
>
> http://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg02234.html

That does look cool, but it's not in the tree yet.

Otherwise can we directly just call tcg_out_brcond()?

PS: Thanks for your review. I have gone through most of your comments.
I now don't see any segfaults when running. My guest still doesn't
boot, but it's getting further then it used to :)

Alistair

>
>
> r~



reply via email to

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