[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [RFC v1 16/23] riscv: tcg-target: Add slow
From: |
Richard Henderson |
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [RFC v1 16/23] riscv: tcg-target: Add slowpath load and store instructions |
Date: |
Wed, 21 Nov 2018 08:43:08 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 |
On 11/21/18 1:18 AM, Alistair Francis wrote:
> 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.
No, but it'll probably be in tree before this code is. ;-)
I'll put it into my tcg-next-for-4.0 branch, and you can base off that.
> Otherwise can we directly just call tcg_out_brcond()?
Not quite, because there's no label structure. But you can break out
subroutines and use those.
> 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 :)
Excellent.
r~
- [Qemu-riscv] [RFC v1 14/23] riscv: tcg-target: Add the out load and store instructions, (continued)
[Qemu-riscv] [RFC v1 16/23] riscv: tcg-target: Add slowpath load and store instructions, Alistair Francis, 2018/11/15
[Qemu-riscv] [RFC v1 17/23] riscv: tcg-target: Add direct load and store instructions, Alistair Francis, 2018/11/15
[Qemu-riscv] [RFC v1 18/23] riscv: tcg-target: Add the out op decoder, Alistair Francis, 2018/11/15
[Qemu-riscv] [RFC v1 19/23] riscv: tcg-target: Add the prologue generation, Alistair Francis, 2018/11/15
[Qemu-riscv] [RFC v1 20/23] riscv: tcg-target: Add the target init code, Alistair Francis, 2018/11/15