[PATCH] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions

Taylor Simpson posted 1 patch 1 year ago
Failed in applying to current master (apply log)
Maintainers: Taylor Simpson <tsimpson@quicinc.com>
target/hexagon/gen_tcg.h | 24 ++++++++++++++++++++++++
target/hexagon/macros.h  | 18 ++++--------------
2 files changed, 28 insertions(+), 14 deletions(-)
[PATCH] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions
Posted by Taylor Simpson 1 year ago
Most of these are not modelled in QEMU, so save the overhead of
calling a helper.

The only exception is dczeroa.  It assigns to hex_dczero_addr, which
is handled during packet commit.

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/gen_tcg.h | 24 ++++++++++++++++++++++++
 target/hexagon/macros.h  | 18 ++++--------------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/target/hexagon/gen_tcg.h b/target/hexagon/gen_tcg.h
index bcf0cf466a..ca41ca1e41 100644
--- a/target/hexagon/gen_tcg.h
+++ b/target/hexagon/gen_tcg.h
@@ -487,6 +487,19 @@
 #define fGEN_TCG_S2_storerinew_pcr(SHORTCODE) \
     fGEN_TCG_STORE_pcr(2, fSTORE(1, 4, EA, NtN))
 
+/* dczeroa clears the 32 byte cache line at the address given */
+#define fGEN_TCG_Y2_dczeroa(SHORTCODE) SHORTCODE
+
+/* In linux-user mode, these are not modelled, suppress compiler warning */
+#define fGEN_TCG_Y2_dcinva(SHORTCODE) \
+    do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_dccleaninva(SHORTCODE) \
+    do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_dccleana(SHORTCODE) \
+    do { RsV = RsV; } while (0)
+#define fGEN_TCG_Y2_icinva(SHORTCODE) \
+    do { RsV = RsV; } while (0)
+
 /*
  * dealloc_return
  * Assembler mapped to
@@ -1187,6 +1200,17 @@
     do { \
         RsV = RsV; \
     } while (0)
+#define fGEN_TCG_Y2_isync(SHORTCODE) \
+    do { } while (0)
+#define fGEN_TCG_Y2_barrier(SHORTCODE) \
+    do { } while (0)
+#define fGEN_TCG_Y2_syncht(SHORTCODE) \
+    do { } while (0)
+#define fGEN_TCG_Y2_dcfetchbo(SHORTCODE) \
+    do { \
+        RsV = RsV; \
+        uiV = uiV; \
+    } while (0)
 
 #define fGEN_TCG_J2_trap0(SHORTCODE) \
     do { \
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 482a9c787f..d878688d96 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -688,20 +688,10 @@ static inline TCGv gen_read_ireg(TCGv result, TCGv val, int shift)
                    reg_field_info[FIELD].offset)
 #define fGET_FIELD(VAL, FIELD)
 #define fSET_FIELD(VAL, FIELD, NEWVAL)
-#define fBARRIER()
-#define fSYNCH()
-#define fISYNC()
-#define fDCFETCH(REG) \
-    do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fICINVA(REG) \
-    do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fL2FETCH(ADDR, HEIGHT, WIDTH, STRIDE, FLAGS)
-#define fDCCLEANA(REG) \
-    do { (void)REG; } while (0) /* Nothing to do in qemu */
-#define fDCCLEANINVA(REG) \
-    do { (void)REG; } while (0) /* Nothing to do in qemu */
-
-#define fDCZEROA(REG) do { env->dczero_addr = (REG); } while (0)
+
+#ifdef QEMU_GENERATE
+#define fDCZEROA(REG) tcg_gen_mov_tl(hex_dczero_addr, (REG))
+#endif
 
 #define fBRANCH_SPECULATE_STALL(DOTNEWVAL, JUMP_COND, SPEC_DIR, HINTBITNUM, \
                                 STRBITNUM) /* Nothing */
-- 
2.25.1

Re: [PATCH] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions
Posted by Richard Henderson 1 year ago
On 4/10/23 13:24, Taylor Simpson wrote:
> Most of these are not modelled in QEMU, so save the overhead of
> calling a helper.
> 
> The only exception is dczeroa.  It assigns to hex_dczero_addr, which
> is handled during packet commit.
> 
> Signed-off-by: Taylor Simpson<tsimpson@quicinc.com>
> ---
>   target/hexagon/gen_tcg.h | 24 ++++++++++++++++++++++++
>   target/hexagon/macros.h  | 18 ++++--------------
>   2 files changed, 28 insertions(+), 14 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Something to look at in the future: I believe quite a lot of these variables like 
dczero_addr are not "real" architectural state, in that they do not persist beyond the 
lifetime of the packet.  There are others, e.g. pkt_has_store_s1.

These variables could be moved to DisasContext and allocated on demand.  Even recently 
this was tedious, because of TCG temporary lifetime issues, but no longer.

Just a thought.


r~
RE: [PATCH] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions
Posted by Taylor Simpson 1 year ago

> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Monday, April 10, 2023 8:30 PM
> To: Taylor Simpson <tsimpson@quicinc.com>; qemu-devel@nongnu.org
> Cc: philmd@linaro.org; ale@rev.ng; anjo@rev.ng; Brian Cain
> <bcain@quicinc.com>; Matheus Bernardino (QUIC)
> <quic_mathbern@quicinc.com>
> Subject: Re: [PATCH] Hexagon (target/hexagon) Add overrides for
> cache/sync/barrier instructions
> 
> On 4/10/23 13:24, Taylor Simpson wrote:
> > Most of these are not modelled in QEMU, so save the overhead of
> > calling a helper.
> >
> > The only exception is dczeroa.  It assigns to hex_dczero_addr, which
> > is handled during packet commit.
> >
> > Signed-off-by: Taylor Simpson<tsimpson@quicinc.com>
> > ---
> >   target/hexagon/gen_tcg.h | 24 ++++++++++++++++++++++++
> >   target/hexagon/macros.h  | 18 ++++--------------
> >   2 files changed, 28 insertions(+), 14 deletions(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> Something to look at in the future: I believe quite a lot of these variables like
> dczero_addr are not "real" architectural state, in that they do not persist
> beyond the lifetime of the packet.  There are others, e.g. pkt_has_store_s1.

That's correct.

> These variables could be moved to DisasContext and allocated on demand.
> Even recently this was tedious, because of TCG temporary lifetime issues,
> but no longer.

I'll work on this.  The obvious advantage is to allow the TCG optimizer more opportunity to fold copies and propagate constants.

Any other advantage?

Thanks,
Taylor

Re: [PATCH] Hexagon (target/hexagon) Add overrides for cache/sync/barrier instructions
Posted by Richard Henderson 1 year ago
On 4/11/23 04:29, Taylor Simpson wrote:
>> These variables could be moved to DisasContext and allocated on demand.
>> Even recently this was tedious, because of TCG temporary lifetime issues,
>> but no longer.
> 
> I'll work on this.  The obvious advantage is to allow the TCG optimizer more opportunity to fold copies and propagate constants.
> 
> Any other advantage?

Once the packet is complete, the value is discarded, rather than written back to env.


r~