[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
RE: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers
From: |
Taylor Simpson |
Subject: |
RE: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers |
Date: |
Wed, 26 Aug 2020 23:52:07 +0000 |
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, August 26, 2020 8:17 AM
> 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 07/34] Hexagon (target/hexagon) scalar core
> helpers
>
> > +DEF_HELPER_3(merge_inflight_store8u, s64, env, s32, s64)
>
> Please use DEF_HELPER_FLAGS_N when possible.
OK
> > +static inline void log_pred_write(CPUHexagonState *env, int pnum,
> > + target_ulong val)
>
> You might be better off letting the compiler decide about inlining.
Isn't "inline" just a hint to the compiler?
> > + union {
> > + uint8_t bytes[8];
> > + uint32_t data32;
> > + uint64_t data64;
> > + } retdata, storedata;
>
> Red flag here. This is assuming that the host and the target are both
> little-endian.
OK
> > + int bigmask = ((-load_width) & (-store_width));
> > + if ((load_addr & bigmask) != (store_addr & bigmask)) {
>
> Huh? This doesn't detect overlap. Counter example:
>
> load_addr = 0, load_width = 4,
> store_addr = 1, store_width = 1.
>
> bigmask = -4 & -1 -> -4.
>
> (0 & -4) != (1 & -4) -> 0 != 1
OK
> > + while ((i < load_width) && (j < store_width)) {
> > + retdata.bytes[i] = storedata.bytes[j];
> > + i++;
> > + j++;
> > + }
> > + return retdata.data64;
>
> This most definitely requires host-endian adjustment.
OK
> > +/* Helpful for printing intermediate values within instructions */
> > +void HELPER(debug_value)(CPUHexagonState *env, int32_t value)
> > +{
> > + HEX_DEBUG_LOG("value = 0x%x\n", value);
> > +}
> > +
> > +void HELPER(debug_value_i64)(CPUHexagonState *env, int64_t value)
> > +{
> > + HEX_DEBUG_LOG("value_i64 = 0x%lx\n", value);
> > +}
>
> I think we need to lose all of this.
> There are other ways to debug TCG.
OK
- [RFC PATCH v3 03/34] Hexagon (include/elf.h) ELF machine definition, (continued)
- [RFC PATCH v3 03/34] Hexagon (include/elf.h) ELF machine definition, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 13/34] Hexagon (target/hexagon) register map, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 14/34] Hexagon (target/hexagon) instruction/packet decode, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 17/34] Hexagon (target/hexagon/imported) arch import - macro definitions, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 23/34] Hexagon (target/hexagon) generater phase 4 - decode tree, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 09/34] Hexagon (target/hexagon) architecture types, Taylor Simpson, 2020/08/18
- [RFC PATCH v3 16/34] Hexagon (target/hexagon) utility functions, Taylor Simpson, 2020/08/18