[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros referenced in i
From: |
Taylor Simpson |
Subject: |
RE: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros referenced in instruction semantics |
Date: |
Sun, 30 Aug 2020 20:23:20 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Friday, August 28, 2020 7:17 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@redhat.com; laurent@vivier.eu; riku.voipio@iki.fi;
> aleksandar.m.mail@gmail.com; ale@rev.ng
> Subject: Re: [RFC PATCH v3 26/34] Hexagon (target/hexagon) macros
> referenced in instruction semantics
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > + * For qemu, we look for a load in slot 0 when there is a store in slot 1
> > + * in the same packet. When we see this, we call a helper that merges the
> > + * bytes from the store buffer with the value loaded from memory.
> > + */
> > +#define CHECK_NOSHUF(DST, VA, SZ, SIGN) \
> > + do { \
> > + if (insn->slot == 0 && pkt->pkt_has_store_s1) { \
> > + gen_helper_merge_inflight_store##SZ##SIGN(DST, cpu_env, VA,
> DST); \
> > + } \
> > + } while (0)
>
> Ah, so I see what you're trying to do with the merge thing, which had the
> host-endian problems.
>
> I think the merge stuff is a mistake. I think you can get the semantics that
> you want with
>
> probe_read(ld_addr, ld_len)
> qemu_st(st_value, st_addr)
> qemu_ld(ld_value, ld_addr)
>
> In this way, all exceptions are recognized before the store is complete, the
> normal memory operations handle any possible overlap.
So, do this inside the helper? Or is there a way to generate TCG code?
>
> > +#define fINSERT_BITS(REG, WIDTH, OFFSET, INVAL) \
> > + do { \
> > + REG = ((REG) & ~(((fCONSTLL(1) << (WIDTH)) - 1) << (OFFSET))) | \
> > + (((INVAL) & ((fCONSTLL(1) << (WIDTH)) - 1)) << (OFFSET)); \
> > + } while (0)
>
> reg = deposit32(reg, offset, width, inval)
OK
> > +#define fEXTRACTU_BITS(INREG, WIDTH, OFFSET) \
> > + (fZXTN(WIDTH, 32, (INREG >> OFFSET)))
> > +#define fEXTRACTU_BIDIR(INREG, WIDTH, OFFSET) \
> > + (fZXTN(WIDTH, 32, fBIDIR_LSHIFTR((INREG), (OFFSET), 4_8)))
> > +#define fEXTRACTU_RANGE(INREG, HIBIT, LOWBIT) \
> > + (fZXTN((HIBIT - LOWBIT + 1), 32, (INREG >> LOWBIT)))
>
> extract32(inreg, offset, width)
OK
> > +#define fZXTN(N, M, VAL) ((VAL) & ((1LL << (N)) - 1))
>
> extract32(VAL, 0, n)
OK
> > +#define fSXTN(N, M, VAL) \
> > + ((fZXTN(N, M, VAL) ^ (1LL << ((N) - 1))) - (1LL << ((N) - 1)))
>
> sextract32(val, 0, n)
OK
> > +#define fRND(A) (((A) + 1) >> 1)
>
> Does A have a type that won't overflow?
> For Arm we write this as
>
> (A >> 1) + (A & 1)
Will investigate
> > +#define fDCFETCH(REG) do { REG = REG; } while (0) /* Nothing to do in
> qemu */
> > +#define fICINVA(REG) do { REG = REG; } while (0) /* Nothing to do in
> qemu */
> > +#define fL2FETCH(ADDR, HEIGHT, WIDTH, STRIDE, FLAGS)
> > +#define fDCCLEANA(REG) do { REG = REG; } while (0) /* Nothing to do in
> qemu */
> > +#define fDCCLEANINVA(REG) \
> > + do { REG = REG; } while (0) /* Nothing to do in qemu */
>
> Is this "R=R" thing trying to avoid a compiler warning?
> Perhaps "(void)R" would be sufficient to avoid that?
Yes, it avoids a compiler warning. Will change to (void)
> > -static inline void log_store32(CPUHexagonState *env, target_ulong addr,
> > - target_ulong val, int width, int slot)
> > -{
> > - HEX_DEBUG_LOG("log_store%d(0x" TARGET_FMT_lx ", "
> TARGET_FMT_ld
> > - " [0x" TARGET_FMT_lx "])\n",
> > - width, addr, val, val);
> > - env->mem_log_stores[slot].va = addr;
> > - env->mem_log_stores[slot].width = width;
> > - env->mem_log_stores[slot].data32 = val;
> > -}
> > -
> > -static inline void log_store64(CPUHexagonState *env, target_ulong addr,
> > - int64_t val, int width, int slot)
> > -{
> > - HEX_DEBUG_LOG("log_store%d(0x" TARGET_FMT_lx ", %ld [0x%lx])\n",
> > - width, addr, val, val);
> > - env->mem_log_stores[slot].va = addr;
> > - env->mem_log_stores[slot].width = width;
> > - env->mem_log_stores[slot].data64 = val;
> > -}
> > -
>
> Fold this back to wherever it came from. Clearly no need to introduce it in
> the first place.
These are invoked by the MEM_STORE macros. Are you saying to put this code
there?
[RFC PATCH v3 18/34] Hexagon (target/hexagon/imported) arch import - instruction semantics, Taylor Simpson, 2020/08/18
[RFC PATCH v3 19/34] Hexagon (target/hexagon/imported) arch import - instruction encoding, Taylor Simpson, 2020/08/18
[RFC PATCH v3 25/34] Hexagon (target/hexagon) macros to interface with the generator, Taylor Simpson, 2020/08/18