Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes

2015-08-12 Thread Laurent Vivier 


Le 12/08/2015 06:07, Richard Henderson a écrit :
 On 08/09/2015 01:13 PM, Laurent Vivier wrote:
 -#define OS_BYTE 0
 -#define OS_WORD 1
 -#define OS_LONG 2
 -#define OS_SINGLE 4
 -#define OS_DOUBLE 5
 +#define OS_BYTE 1
 +#define OS_WORD 2
 +#define OS_LONG 3
 +#define OS_SINGLE   4
 +#define OS_DOUBLE   5
 +#define OS_EXTENDED 6
 +#define OS_PACKED   7

 
 Is there a reason you've skipped the 0 value when adding the new values?

I think there is no reason, but if I change the value I have to update
abdc_mem, sbcd_mem instructions as they use it as an
incrementer/decrementer. I agree, it's a strange idea.

 
 +static inline int insn_opsize(int insn, int pos)
 +{
 +switch ((insn  pos)  3) {
 
 
 In particular, that change means that insn_opsize is more complicated
 than needed.  Further, is there any reason for POS to be a varable? 
 Isn't it at the same place for all insns?
 
 +static inline int ext_opsize(int ext, int pos)
 
 This should probably wait until the fp insns get added.

Yes.

Laurent

Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes

2015-08-12 Thread Andreas Schwab 
Laurent Vivier laur...@vivier.eu writes:

 Le 12/08/2015 06:07, Richard Henderson a écrit :
 Is there a reason you've skipped the 0 value when adding the new values?

 I think there is no reason, but if I change the value I have to update
 abdc_mem, sbcd_mem instructions as they use it as an
 incrementer/decrementer. I agree, it's a strange idea.

Those uses are really opsize_bytes(OS_BYTE), technically.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.

Re: [Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes

2015-08-11 Thread Richard Henderson 

On 08/09/2015 01:13 PM, Laurent Vivier wrote:

-#define OS_BYTE 0
-#define OS_WORD 1
-#define OS_LONG 2
-#define OS_SINGLE 4
-#define OS_DOUBLE 5
+#define OS_BYTE 1
+#define OS_WORD 2
+#define OS_LONG 3
+#define OS_SINGLE   4
+#define OS_DOUBLE   5
+#define OS_EXTENDED 6
+#define OS_PACKED   7



Is there a reason you've skipped the 0 value when adding the new values?


+static inline int insn_opsize(int insn, int pos)
+{
+switch ((insn  pos)  3) {



In particular, that change means that insn_opsize is more complicated than 
needed.  Further, is there any reason for POS to be a varable?  Isn't it at the 
same place for all insns?



+static inline int ext_opsize(int ext, int pos)


This should probably wait until the fp insns get added.


r~

[Qemu-devel] [PATCH for-2.5 05/30] m68k: define operand sizes

2015-08-09 Thread Laurent Vivier 
Signed-off-by: Laurent Vivier laur...@vivier.eu
---
 target-m68k/translate.c | 78 +
 1 file changed, 47 insertions(+), 31 deletions(-)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 6ba71a2..eb7f503 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -146,11 +146,13 @@ typedef struct DisasContext {
 static void *gen_throws_exception;
 #define gen_last_qop NULL
 
-#define OS_BYTE 0
-#define OS_WORD 1
-#define OS_LONG 2
-#define OS_SINGLE 4
-#define OS_DOUBLE 5
+#define OS_BYTE 1
+#define OS_WORD 2
+#define OS_LONG 3
+#define OS_SINGLE   4
+#define OS_DOUBLE   5
+#define OS_EXTENDED 6
+#define OS_PACKED   7
 
 typedef void (*disas_proc)(CPUM68KState *env, DisasContext *s, uint16_t insn);
 
@@ -446,11 +448,49 @@ static inline int opsize_bytes(int opsize)
 case OS_LONG: return 4;
 case OS_SINGLE: return 4;
 case OS_DOUBLE: return 8;
+case OS_EXTENDED: return 12;
+case OS_PACKED: return 12;
 default:
 g_assert_not_reached();
 }
 }
 
+static inline int insn_opsize(int insn, int pos)
+{
+switch ((insn  pos)  3) {
+case 0:
+return OS_BYTE;
+case 1:
+return OS_WORD;
+case 2:
+return OS_LONG;
+default:
+abort();
+}
+}
+
+static inline int ext_opsize(int ext, int pos)
+{
+switch ((ext  pos)  7) {
+case 0:
+return OS_LONG;
+case 1:
+return OS_SINGLE;
+case 2:
+return OS_EXTENDED;
+case 3:
+return OS_PACKED;
+case 4:
+return OS_WORD;
+case 5:
+return OS_DOUBLE;
+case 6:
+return OS_BYTE;
+default:
+abort();
+}
+}
+
 /* Assign value to a register.  If the width is less than the register width
only the low part of the register is set.  */
 static void gen_partset_reg(int opsize, TCGv reg, TCGv val)
@@ -1321,19 +1361,7 @@ DISAS_INSN(clr)
 {
 int opsize;
 
-switch ((insn  6)  3) {
-case 0: /* clr.b */
-opsize = OS_BYTE;
-break;
-case 1: /* clr.w */
-opsize = OS_WORD;
-break;
-case 2: /* clr.l */
-opsize = OS_LONG;
-break;
-default:
-abort();
-}
+opsize = insn_opsize(insn, 6);
 DEST_EA(env, insn, opsize, tcg_const_i32(0), NULL);
 gen_logic_cc(s, tcg_const_i32(0));
 }
@@ -1477,19 +1505,7 @@ DISAS_INSN(tst)
 int opsize;
 TCGv tmp;
 
-switch ((insn  6)  3) {
-case 0: /* tst.b */
-opsize = OS_BYTE;
-break;
-case 1: /* tst.w */
-opsize = OS_WORD;
-break;
-case 2: /* tst.l */
-opsize = OS_LONG;
-break;
-default:
-abort();
-}
+opsize = insn_opsize(insn, 6);
 SRC_EA(env, tmp, opsize, 1, NULL);
 gen_logic_cc(s, tmp);
 }
-- 
2.4.3