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

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

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 <peter.maydell@linaro.org>
> ---
> 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;
> +            /*
> +             * 0b1111_1010_0xxx_xxxx_1111_xxxx_0000_xxxx:
> +             *  - 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_xxxx_xxxx
> +             *  - 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_xxxx_xxxx_xxxx
> +         *  - 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_xxxx_xxxx
> +         *  - Data-processing (two low registers)

T1, OK

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

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>