RE: [RFC PATCH v3 28/34] Hexagon (target/hexagon) TCG generation helpers

[Taylor Simpson]

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Sunday, August 30, 2020 2:52 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 28/34] Hexagon (target/hexagon) TCG
> generation helpers
>
> On 8/30/20 12:53 PM, Taylor Simpson wrote:
> >>> +++ b/target/hexagon/genptr_helpers.h
> >>> @@ -0,0 +1,244 @@
> >>> +
> >>> +static inline void gen_log_reg_write(int rnum, TCGv val, int slot,
> >>> +                                     int is_predicated)
> >>
> >> These are quite large.  Why are they marked inline?
> >
> > Since this is a header file, it prevents the compiler from complaining when
> they aren't used.
>
> Ok, why are they in a header file?
> Why would they be unused, come to that?
>
> The header file is used exactly once, by genptr.c.  Seems to me they could
> just
> as well be *in* genptr.c.
>
> If the functions are not used, just remove them?

I could have sworn it was included in more than one C file.  I'll move the 
contents to genptr.c.


> >>> +static inline void log_store32(CPUHexagonState *env, target_ulong
> addr,
> >>> +                               int32_t val, int width, int slot)
> >>> +{
> >>> +    HEX_DEBUG_LOG("log_store%d(0x%x, %d [0x%x])\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%x, %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;
> >>> +}
> >>
> >> ... or fold this re-addition back into where it was accidentally removed.  
> >> ;-)
> >
> > Could you elaborate?
>
> You added this code in one patch (didn't check which), removed it in patch
> 26,
> and re-added it here in patch 28.

My apologies, this is my screwing up the git rebase.  I'll fix it.

>
>
> r~