Skip to contentSkip to conversation

convert SPE logical instructions to TCG

PATCH In review Nathan Froyd 4 participants 6 messages Oct 14, 2008 – Oct 15, 2008

lore
Share & tools
lore

Nathan Froyd

author
#0 lore raw

PATCH 1/1 convert SPE logical instructions to TCG

As the subject suggests. I was unsure of whether to use the cpu_T array or just create new TCG locals; I opted for the latter approach, since that approach seems a little more in spirit with how TCG is supposed to be used, even if it's at odds with the rest of the PPC backend. If this decision is a poor one, let me know and I can update the patch.

-Nathan

op.c +0 -48
Index: target-ppc/op.c
===================================================================
@@ -2513,54 +2513,6 @@
     RETURN();
 }
 
-void OPPROTO op_evand (void)
-{
-    T0_64 &= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evandc (void)
-{
-    T0_64 &= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evor (void)
-{
-    T0_64 |= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evxor (void)
-{
-    T0_64 ^= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_eveqv (void)
-{
-    T0_64 = ~(T0_64 ^ T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evnor (void)
-{
-    T0_64 = ~(T0_64 | T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evorc (void)
-{
-    T0_64 |= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evnand (void)
-{
-    T0_64 = ~(T0_64 & T1_64);
-    RETURN();
-}
-
 void OPPROTO op_evsrws (void)
 {
     do_evsrws();
translate.c +133 -8

147 lines not on this page · raw message

#1 lore raw

I think (keyword think) that the use of cpu_T is used for backwards compatibility with dyngen. As soon as everything is ported to TCG, expect the cpu_T functions to go away.

Quoted 8 lines · Nathan Froyd

As the subject suggests. I was unsure of whether to use the cpu_T array or just create new TCG locals; I opted for the latter approach, since that approach seems a little more in spirit with how TCG is supposed to be used, even if it's at odds with the rest of the PPC backend. If this decision is a poor one, let me know and I can update the patch.

-Nathan

#2 lore raw
In reply to Nathan Froyd
Quoted 5 lines · Nathan Froyd

As the subject suggests. I was unsure of whether to use the cpu_T array or just create new TCG locals; I opted for the latter approach, since that approach seems a little more in spirit with how TCG is supposed to be used, even if it's at odds with the rest of the PPC backend. If this decision is a poor one, let me know and I can update the patch.

Yes, you should use local variable, cpu_T array will eventually disappear. However, I am more concerned by the fact that it may not be a good idea to expand the macro. IMHO you should put the common code in the macro and pass the name of the function that does the logical operation as an argument.

Aurelien

Quoted 214 lines

Index: target-ppc/op.c
===================================================================

op.c +0 -48
@@ -2513,54 +2513,6 @@
     RETURN();
 }
 
-void OPPROTO op_evand (void)
-{
-    T0_64 &= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evandc (void)
-{
-    T0_64 &= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evor (void)
-{
-    T0_64 |= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evxor (void)
-{
-    T0_64 ^= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_eveqv (void)
-{
-    T0_64 = ~(T0_64 ^ T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evnor (void)
-{
-    T0_64 = ~(T0_64 | T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evorc (void)
-{
-    T0_64 |= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evnand (void)
-{
-    T0_64 = ~(T0_64 & T1_64);
-    RETURN();
-}
-
 void OPPROTO op_evsrws (void)
 {
     do_evsrws();

Index: target-ppc/translate.c
===================================================================

translate.c +133 -8
@@ -5453,14 +5453,139 @@
 }
 
 /* Logical */
-GEN_SPEOP_ARITH2(evand);
-GEN_SPEOP_ARITH2(evandc);
-GEN_SPEOP_ARITH2(evxor);
-GEN_SPEOP_ARITH2(evor);
-GEN_SPEOP_ARITH2(evnor);
-GEN_SPEOP_ARITH2(eveqv);
-GEN_SPEOP_ARITH2(evorc);
-GEN_SPEOP_ARITH2(evnand);
+static always_inline void gen_evand (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_and_i64(t0, t0, t1);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evandc (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_not_i64(t1, t1);
+    tcg_gen_and_i64(t0, t0, t1);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evxor (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_xor_i64(t0, t0, t1);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evor (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_or_i64(t0, t0, t1);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evnor (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_or_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_eveqv (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_xor_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evorc (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_not_i64(t1, t1);
+    tcg_gen_or_i64(t0, t0, t1);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
+static always_inline void gen_evnand (DisasContext *ctx)
+{
+    if (unlikely(!ctx->spe_enabled)) {
+        GEN_EXCP_NO_AP(ctx);
+        return;
+    }
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);
+    gen_load_gpr64(t0, rA(ctx->opcode));
+    gen_load_gpr64(t1, rB(ctx->opcode));
+    tcg_gen_and_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+    gen_store_gpr64(rD(ctx->opcode), t0);
+    tcg_temp_free(t0);
+    tcg_temp_free(t1);
+}
+
 GEN_SPEOP_ARITH2(evsrwu);
 GEN_SPEOP_ARITH2(evsrws);
 GEN_SPEOP_ARITH2(evslw);
Signature · 4 lines
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net
author
#3 lore raw
Quoted 5 lines · Aurelien Jarno

Yes, you should use local variable, cpu_T array will eventually disappear. However, I am more concerned by the fact that it may not be a good idea to expand the macro. IMHO you should put the common code in the macro and pass the name of the function that does the logical operation as an argument.

Done thusly.

-Nathan

target-ppc/op.c +0 -48
diff -r 5ed4e3fd0fe7 -r cac4af009435 target-ppc/op.c
@@ -2513,54 +2513,6 @@
     RETURN();
 }
 
-void OPPROTO op_evand (void)
-{
-    T0_64 &= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evandc (void)
-{
-    T0_64 &= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evor (void)
-{
-    T0_64 |= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evxor (void)
-{
-    T0_64 ^= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_eveqv (void)
-{
-    T0_64 = ~(T0_64 ^ T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evnor (void)
-{
-    T0_64 = ~(T0_64 | T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evorc (void)
-{
-    T0_64 |= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evnand (void)
-{
-    T0_64 = ~(T0_64 & T1_64);
-    RETURN();
-}
-
 void OPPROTO op_evsrws (void)
 {
     do_evsrws();
target-ppc/translate.c +70 -8

90 lines not on this page · raw message

#4 lore raw
Quoted 2 lines · On Wednesday 15 October 2008 03:09:16 Nathan Froyd wrote:

+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);
 +    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);

These should be tcg_temp_new.

Local variables (tcg_temp_local_new) introduce a lot of additional overhead, should only be used when absolutely necessary. A local variable is required if the value is required to live over multiple basic blocks. i.e. accross a branch op or a dyngen op. In the latter case it's probably best to eliminate the dyngen op.

Paul

#5 lore raw
In reply to Nathan Froyd
Quoted 6 lines · Nathan Froyd
Quoted 5 lines · Aurelien Jarno

Yes, you should use local variable, cpu_T array will eventually disappear. However, I am more concerned by the fact that it may not be a good idea to expand the macro. IMHO you should put the common code in the macro and pass the name of the function that does the logical operation as an argument.

Applied with the change suggested by Paul Brook. Thanks.

Quoted 160 lines

Done thusly.

-Nathan

diff -r 5ed4e3fd0fe7 -r cac4af009435 target-ppc/op.c

target-ppc/op.c +0 -48
@@ -2513,54 +2513,6 @@
     RETURN();
 }
 
-void OPPROTO op_evand (void)
-{
-    T0_64 &= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evandc (void)
-{
-    T0_64 &= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evor (void)
-{
-    T0_64 |= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evxor (void)
-{
-    T0_64 ^= T1_64;
-    RETURN();
-}
-
-void OPPROTO op_eveqv (void)
-{
-    T0_64 = ~(T0_64 ^ T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evnor (void)
-{
-    T0_64 = ~(T0_64 | T1_64);
-    RETURN();
-}
-
-void OPPROTO op_evorc (void)
-{
-    T0_64 |= ~T1_64;
-    RETURN();
-}
-
-void OPPROTO op_evnand (void)
-{
-    T0_64 = ~(T0_64 & T1_64);
-    RETURN();
-}
-
 void OPPROTO op_evsrws (void)
 {
     do_evsrws();

diff -r 5ed4e3fd0fe7 -r cac4af009435 target-ppc/translate.c

target-ppc/translate.c +70 -8
@@ -5427,6 +5427,23 @@
     gen_store_gpr64(rD(ctx->opcode), cpu_T64[0]);                             \
 }
 
+#define GEN_SPEOP_TCG_ARITH2(name)                                            \
+static always_inline void gen_##name (DisasContext *ctx)                      \
+{                                                                             \
+    if (unlikely(!ctx->spe_enabled)) {                                        \
+        GEN_EXCP_NO_AP(ctx);                                                  \
+        return;                                                               \
+    }                                                                         \
+    TCGv t0 = tcg_temp_local_new(TCG_TYPE_I64);                               \
+    TCGv t1 = tcg_temp_local_new(TCG_TYPE_I64);                               \
+    gen_load_gpr64(t0, rA(ctx->opcode));                                      \
+    gen_load_gpr64(t1, rB(ctx->opcode));                                      \
+    gen_op_##name(t0, t1);                                                    \
+    gen_store_gpr64(rD(ctx->opcode), t0);                                     \
+    tcg_temp_free(t0);                                                        \
+    tcg_temp_free(t1);                                                        \
+}
+
 #define GEN_SPEOP_ARITH1(name)                                                \
 static always_inline void gen_##name (DisasContext *ctx)                      \
 {                                                                             \

@@ -5453,14 +5470,59 @@
 }
 
 /* Logical */
-GEN_SPEOP_ARITH2(evand);
-GEN_SPEOP_ARITH2(evandc);
-GEN_SPEOP_ARITH2(evxor);
-GEN_SPEOP_ARITH2(evor);
-GEN_SPEOP_ARITH2(evnor);
-GEN_SPEOP_ARITH2(eveqv);
-GEN_SPEOP_ARITH2(evorc);
-GEN_SPEOP_ARITH2(evnand);
+static always_inline void gen_op_evand (TCGv t0, TCGv t1)
+{
+    tcg_gen_and_i64(t0, t0, t1);
+}
+
+static always_inline void gen_op_evandc (TCGv t0, TCGv t1)
+{
+    tcg_gen_not_i64(t1, t1);
+    tcg_gen_and_i64(t0, t0, t1);
+}
+
+static always_inline void gen_op_evxor (TCGv t0, TCGv t1)
+{
+    tcg_gen_xor_i64(t0, t0, t1);
+}
+
+static always_inline void gen_op_evor (TCGv t0, TCGv t1)
+{
+    tcg_gen_or_i64(t0, t0, t1);
+}
+
+static always_inline void gen_op_evnor (TCGv t0, TCGv t1)
+{
+    tcg_gen_or_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+}
+
+static always_inline void gen_op_eveqv (TCGv t0, TCGv t1)
+{
+    tcg_gen_xor_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+}
+
+static always_inline void gen_op_evorc (TCGv t0, TCGv t1)
+{
+    tcg_gen_not_i64(t1, t1);
+    tcg_gen_or_i64(t0, t0, t1);
+}
+
+static always_inline void gen_op_evnand (TCGv t0, TCGv t1)
+{
+    tcg_gen_and_i64(t0, t0, t1);
+    tcg_gen_not_i64(t0, t0);
+}
+
+GEN_SPEOP_TCG_ARITH2(evand);
+GEN_SPEOP_TCG_ARITH2(evandc);
+GEN_SPEOP_TCG_ARITH2(evxor);
+GEN_SPEOP_TCG_ARITH2(evor);
+GEN_SPEOP_TCG_ARITH2(evnor);
+GEN_SPEOP_TCG_ARITH2(eveqv);
+GEN_SPEOP_TCG_ARITH2(evorc);
+GEN_SPEOP_TCG_ARITH2(evnand);
 GEN_SPEOP_ARITH2(evsrwu);
 GEN_SPEOP_ARITH2(evsrws);
 GEN_SPEOP_ARITH2(evslw);
Signature · 4 lines
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   [email protected]         | [email protected]
   `-    people.debian.org/~aurel32 | www.aurel32.net