From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:34650)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1XkDhZ-0000zK-Fh
	for qemu-devel@nongnu.org; Fri, 31 Oct 2014 11:01:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1XkCY4-0006nN-W1
	for qemu-devel@nongnu.org; Fri, 31 Oct 2014 09:47:36 -0400
Received: from static.88-198-71-155.clients.your-server.de
	([88.198.71.155]:46015 helo=socrates.bennee.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <alex.bennee@linaro.org>) id 1XkCY0-0006cR-FG
	for qemu-devel@nongnu.org; Fri, 31 Oct 2014 09:47:28 -0400
References: <1414524244-20316-1-git-send-email-peter.maydell@linaro.org>
	<1414524244-20316-6-git-send-email-peter.maydell@linaro.org>
From: Alex =?utf-8?Q?Benn=C3=A9e?= <alex.bennee@linaro.org>
In-reply-to: <1414524244-20316-6-git-send-email-peter.maydell@linaro.org>
Date: Fri, 31 Oct 2014 13:47:18 +0000
Message-ID: <87vbn02vx5.fsf@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH 5/5] target-arm/translate.c: Don't pass
	CPUARMState * to disas_arm_insn()
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, patches@linaro.org


Peter Maydell <peter.maydell@linaro.org> writes:

> Refactor to avoid passing a CPUARMState * to disas_arm_insn(). To do this
> we move the "read insn from memory" code to the callsite and pass the
> insn to the function instead.
>
<snip>
>  
> -static void disas_arm_insn(CPUARMState * env, DisasContext *s)
> +static void disas_arm_insn(DisasContext *s, unsigned int insn)

I note that in the aarch64 code we used the unambiguous uint32_t for the
insn type. I'm hard pressed to imagine it actually breaking anything
though.



>  {
> -    unsigned int cond, insn, val, op1, i, shift, rm, rs, rn, rd, sh;
> +    unsigned int cond, val, op1, i, shift, rm, rs, rn, rd, sh;
>      TCGv_i32 tmp;
>      TCGv_i32 tmp2;
>      TCGv_i32 tmp3;
>      TCGv_i32 addr;
>      TCGv_i64 tmp64;
>  
> -    insn = arm_ldl_code(env, s->pc, s->bswap_code);
> -    s->pc += 4;
> -
>      /* M variants do not implement ARM mode.  */
>      if (arm_dc_feature(s, ARM_FEATURE_M)) {
>          goto illegal_op;
> @@ -11199,7 +11196,9 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
>                  }
>              }
>          } else {
> -            disas_arm_insn(env, dc);
> +            unsigned int insn = arm_ldl_code(env, dc->pc, dc->bswap_code);
> +            dc->pc += 4;
> +            disas_arm_insn(dc, insn);
>          }
>  
>          if (dc->condjmp && !dc->is_jmp) {

Anyway looks fine:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée