Re: [Qemu-devel] [PATCH 05/13] target/arm: Add some comments in Thumb decode

[Richard Henderson]

On 10/2/18 11:35 AM, Peter Maydell wrote:
> Add some comments to the Thumb decoder indicating what bits
> of the instruction have been decoded at various points in
> the code.
> 
> This is not an exhaustive set of comments; we're gradually
> adding comments as we work with particular bits of the code.
> 
> Signed-off-by: Peter Maydell 
> ---
> Specifically, I figured these out as I was going through looking
> for the insns which write SP. These comments turn out not to
> be relevant to those instructions, but I don't want to throw
> them away.
> ---
>  target/arm/translate.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [Qemu-devel] [PATCH 05/13] target/arm: Add some comments in Thumb decode

[Philippe Mathieu-Daudé]

On 02/10/2018 18:35, Peter Maydell wrote:
> Add some comments to the Thumb decoder indicating what bits
> of the instruction have been decoded at various points in
> the code.
> 
> This is not an exhaustive set of comments; we're gradually
> adding comments as we work with particular bits of the code.
> 
> Signed-off-by: Peter Maydell 
> ---
> Specifically, I figured these out as I was going through looking
> for the insns which write SP. These comments turn out not to
> be relevant to those instructions, but I don't want to throw
> them away.
> ---
>  target/arm/translate.c | 20 +---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 25a8fe672f5..fcb33b8a503 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -10623,6 +10623,10 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>  tmp2 = load_reg(s, rm);
>  if ((insn & 0x70) != 0)
>  goto illegal_op;
> +/*
> + * 0b_1010_0xxx_____:
> + *  - MOV, MOVS (register-shifted register), flagsetting

T2, OK

> + */
>  op = (insn >> 21) & 3;
>  logic_cc = (insn & (1 << 20)) != 0;
>  gen_arm_shift_reg(tmp, op, tmp2, logic_cc);
> @@ -11674,7 +11678,11 @@ static void disas_thumb_insn(DisasContext *s, 
> uint32_t insn)
>  rd = insn & 7;
>  op = (insn >> 11) & 3;
>  if (op == 3) {
> -/* add/subtract */
> +/*
> + * 0b0001_1xxx__
> + *  - Add, subtract (three low registers)
> + *  - Add, subtract (two low registers and immediate)

T1, OK

> + */
>  rn = (insn >> 3) & 7;
>  tmp = load_reg(s, rn);
>  if (insn & (1 << 10)) {
> @@ -11711,7 +11719,10 @@ static void disas_thumb_insn(DisasContext *s, 
> uint32_t insn)
>  }
>  break;
>  case 2: case 3:
> -/* arithmetic large immediate */
> +/*
> + * 0b001x___
> + *  - Add, subtract, compare, move (one low register and immediate)

T2, OK

> + */
>  op = (insn >> 11) & 3;
>  rd = (insn >> 8) & 0x7;
>  if (op == 0) { /* mov */
> @@ -11848,7 +11859,10 @@ static void disas_thumb_insn(DisasContext *s, 
> uint32_t insn)
>  break;
>  }
>  
> -/* data processing register */
> +/*
> + * 0b0100_00xx__
> + *  - Data-processing (two low registers)

T1, OK

> + */
>  rd = insn & 7;
>  rm = (insn >> 3) & 7;
>  op = (insn >> 6) & 0xf;
> 

Reviewed-by: Philippe Mathieu-Daudé 

[Qemu-devel] [PATCH 05/13] target/arm: Add some comments in Thumb decode

[Peter Maydell]

Add some comments to the Thumb decoder indicating what bits
of the instruction have been decoded at various points in
the code.

This is not an exhaustive set of comments; we're gradually
adding comments as we work with particular bits of the code.

Signed-off-by: Peter Maydell 
---
Specifically, I figured these out as I was going through looking
for the insns which write SP. These comments turn out not to
be relevant to those instructions, but I don't want to throw
them away.
---
 target/arm/translate.c | 20 +---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 25a8fe672f5..fcb33b8a503 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -10623,6 +10623,10 @@ static void disas_thumb2_insn(DisasContext *s, 
uint32_t insn)
 tmp2 = load_reg(s, rm);
 if ((insn & 0x70) != 0)
 goto illegal_op;
+/*
+ * 0b_1010_0xxx_____:
+ *  - MOV, MOVS (register-shifted register), flagsetting
+ */
 op = (insn >> 21) & 3;
 logic_cc = (insn & (1 << 20)) != 0;
 gen_arm_shift_reg(tmp, op, tmp2, logic_cc);
@@ -11674,7 +11678,11 @@ static void disas_thumb_insn(DisasContext *s, uint32_t 
insn)
 rd = insn & 7;
 op = (insn >> 11) & 3;
 if (op == 3) {
-/* add/subtract */
+/*
+ * 0b0001_1xxx__
+ *  - Add, subtract (three low registers)
+ *  - Add, subtract (two low registers and immediate)
+ */
 rn = (insn >> 3) & 7;
 tmp = load_reg(s, rn);
 if (insn & (1 << 10)) {
@@ -11711,7 +11719,10 @@ static void disas_thumb_insn(DisasContext *s, uint32_t 
insn)
 }
 break;
 case 2: case 3:
-/* arithmetic large immediate */
+/*
+ * 0b001x___
+ *  - Add, subtract, compare, move (one low register and immediate)
+ */
 op = (insn >> 11) & 3;
 rd = (insn >> 8) & 0x7;
 if (op == 0) { /* mov */
@@ -11848,7 +11859,10 @@ static void disas_thumb_insn(DisasContext *s, uint32_t 
insn)
 break;
 }
 
-/* data processing register */
+/*
+ * 0b0100_00xx__
+ *  - Data-processing (two low registers)
+ */
 rd = insn & 7;
 rm = (insn >> 3) & 7;
 op = (insn >> 6) & 0xf;
-- 
2.19.0