diff mbox

[v2] target-mips: generate fences

Message ID 1473328861-15792-1-git-send-email-leon.alrae@imgtec.com (mailing list archive)
State New, archived
Headers show

Commit Message

Leon Alrae Sept. 8, 2016, 10:01 a.m. UTC
Make use of memory barrier TCG opcode in MIPS front end.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
v2:
* generate weaker barriers according to stype
---
 target-mips/translate.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

Comments

Richard Henderson Sept. 8, 2016, 5:04 p.m. UTC | #1
On 09/08/2016 03:01 AM, Leon Alrae wrote:
> Make use of memory barrier TCG opcode in MIPS front end.
> 
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
> v2:
> * generate weaker barriers according to stype
> ---
>  target-mips/translate.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)

Ah, even better.

Reviewed-by: Richard Henderson <rth@twiddle.net>

> +static void gen_sync(int stype)
> +{
> +    TCGOrder tcg_mo = TCG_BAR_SC;
> +
> +    switch (stype) {
> +    case 0x4: /* SYNC_WMB */
> +        tcg_mo |= TCG_MO_ST_ST;
> +        break;
> +    case 0x10: /* SYNC_MB */
> +        tcg_mo |= TCG_MO_ALL;
> +        break;
> +    case 0x11: /* SYNC_ACQUIRE */
> +        tcg_mo |= TCG_MO_LD_LD | TCG_MO_LD_ST;
> +        break;
> +    case 0x12: /* SYNC_RELEASE */
> +        tcg_mo |= TCG_MO_ST_ST | TCG_MO_LD_ST;
> +        break;
> +    case 0x13: /* SYNC_RMB */
> +        tcg_mo |= TCG_MO_LD_LD;
> +        break;
> +    default:
> +        tcg_mo |= TCG_MO_ALL;
> +        break;
> +    }

See git://github.com/rth7680/qemu.git tcg-next wherein this suggests that some
enhancement is desired in tcg/mips/ as well.  Not that I have a mips32r6 board
with which to test.  Would you or James Hogan be able to improve that?


r~
Leon Alrae Sept. 9, 2016, 8:49 a.m. UTC | #2
On Thu, Sep 08, 2016 at 10:04:05AM -0700, Richard Henderson wrote:
> > +static void gen_sync(int stype)
> > +{
> > +    TCGOrder tcg_mo = TCG_BAR_SC;
> > +
> > +    switch (stype) {
> > +    case 0x4: /* SYNC_WMB */
> > +        tcg_mo |= TCG_MO_ST_ST;
> > +        break;
> > +    case 0x10: /* SYNC_MB */
> > +        tcg_mo |= TCG_MO_ALL;
> > +        break;
> > +    case 0x11: /* SYNC_ACQUIRE */
> > +        tcg_mo |= TCG_MO_LD_LD | TCG_MO_LD_ST;
> > +        break;
> > +    case 0x12: /* SYNC_RELEASE */
> > +        tcg_mo |= TCG_MO_ST_ST | TCG_MO_LD_ST;
> > +        break;
> > +    case 0x13: /* SYNC_RMB */
> > +        tcg_mo |= TCG_MO_LD_LD;
> > +        break;
> > +    default:
> > +        tcg_mo |= TCG_MO_ALL;
> > +        break;
> > +    }
> 
> See git://github.com/rth7680/qemu.git tcg-next wherein this suggests that some
> enhancement is desired in tcg/mips/ as well.  Not that I have a mips32r6 board
> with which to test.  Would you or James Hogan be able to improve that?

Actually the lightweight variants of SYNC exist also in earlier revisions of
MIPS architecture; therefore this can be tested for example on mips32r2.

According to manuals the support is optional, and if given CPU doesn't
implement lightweight SYNCs (i.e. with stype != 0) then they are supposed
to behave in the same way as SYNC 0. (which also means I simplified that here
and always interpret the stype to take advantage of weaker ordering barriers).

Thanks,
Leon
Richard Henderson Sept. 12, 2016, 5:02 p.m. UTC | #3
On 09/09/2016 01:49 AM, Leon Alrae wrote:
> Actually the lightweight variants of SYNC exist also in earlier revisions of
> MIPS architecture; therefore this can be tested for example on mips32r2.
>
> According to manuals the support is optional, and if given CPU doesn't
> implement lightweight SYNCs (i.e. with stype != 0) then they are supposed
> to behave in the same way as SYNC 0. (which also means I simplified that here
> and always interpret the stype to take advantage of weaker ordering barriers).

Noted.  I've adjusted the patch in the tcg-next tree to always generate the 
weaker forms of sync.


r~
diff mbox

Patch

diff --git a/target-mips/translate.c b/target-mips/translate.c
index c212e4f..1e66274 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -13109,6 +13109,34 @@  static void gen_ldst_pair (DisasContext *ctx, uint32_t opc, int rd,
     tcg_temp_free(t1);
 }
 
+static void gen_sync(int stype)
+{
+    TCGOrder tcg_mo = TCG_BAR_SC;
+
+    switch (stype) {
+    case 0x4: /* SYNC_WMB */
+        tcg_mo |= TCG_MO_ST_ST;
+        break;
+    case 0x10: /* SYNC_MB */
+        tcg_mo |= TCG_MO_ALL;
+        break;
+    case 0x11: /* SYNC_ACQUIRE */
+        tcg_mo |= TCG_MO_LD_LD | TCG_MO_LD_ST;
+        break;
+    case 0x12: /* SYNC_RELEASE */
+        tcg_mo |= TCG_MO_ST_ST | TCG_MO_LD_ST;
+        break;
+    case 0x13: /* SYNC_RMB */
+        tcg_mo |= TCG_MO_LD_LD;
+        break;
+    default:
+        tcg_mo |= TCG_MO_ALL;
+        break;
+    }
+
+    tcg_gen_mb(tcg_mo);
+}
+
 static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
 {
     int extension = (ctx->opcode >> 6) & 0x3f;
@@ -13384,7 +13412,7 @@  static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
     case 0x2d:
         switch (minor) {
         case SYNC:
-            /* NOP */
+            gen_sync(extract32(ctx->opcode, 16, 5));
             break;
         case SYSCALL:
             generate_exception_end(ctx, EXCP_SYSCALL);
@@ -17201,7 +17229,7 @@  static void decode_opc_special(CPUMIPSState *env, DisasContext *ctx)
         break;
     case OPC_SYNC:
         check_insn(ctx, ISA_MIPS2);
-        /* Treat as NOP. */
+        gen_sync(extract32(ctx->opcode, 6, 5));
         break;
 
 #if defined(TARGET_MIPS64)