From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47764) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPd5L-0008Fs-0T for qemu-devel@nongnu.org; Wed, 12 Aug 2015 16:57:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPd5F-000606-9T for qemu-devel@nongnu.org; Wed, 12 Aug 2015 16:57:18 -0400 Received: from mout.kundenserver.de ([212.227.17.10]:52693) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPd5E-0005zp-WC for qemu-devel@nongnu.org; Wed, 12 Aug 2015 16:57:13 -0400 References: <1439151229-27747-1-git-send-email-laurent@vivier.eu> <1439151229-27747-9-git-send-email-laurent@vivier.eu> <55CAD5D2.5050200@twiddle.net> From: Laurent Vivier Message-ID: <55CBB31B.9030004@vivier.eu> Date: Wed, 12 Aug 2015 22:56:59 +0200 MIME-Version: 1.0 In-Reply-To: <55CAD5D2.5050200@twiddle.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH for-2.5 08/30] m68k: update CPU flags management List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Richard Henderson , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, Andreas Schwab , gerg@uclinux.org Le 12/08/2015 07:12, Richard Henderson a écrit : > On 08/09/2015 01:13 PM, Laurent Vivier wrote: >> @@ -798,9 +796,9 @@ void HELPER(mac_set_flags)(CPUM68KState *env, >> uint32_t acc) >> } >> } >> >> -void HELPER(flush_flags)(CPUM68KState *env, uint32_t cc_op) >> +uint32_t HELPER(flush_flags)(CPUM68KState *env, uint32_t op) >> { >> - cpu_m68k_flush_flags(env, cc_op); >> + return cpu_m68k_flush_flags(env, op); >> } > > Since this function no longer modifies ENV, it probably deserves a > better name than "flush_flags". FWIW cc_compute_all isn't a bad name, > if you're copying i386 anyway... > >> -DEF_HELPER_2(flush_flags, void, env, i32) >> +DEF_HELPER_2(flush_flags, i32, env, i32) > > Modify to use DEF_HELPER_FLAGS while you're at it. At the moment it > reads some globals, but doesn't write any, or have any other side effects. It writes "env->cc_x", so I guess I can't use DEF_HELPER_FLAGS ? > >> static inline void gen_flush_flags(DisasContext *s) >> { >> if (s->cc_op == CC_OP_FLAGS) >> return; >> - gen_flush_cc_op(s); >> - gen_helper_flush_flags(cpu_env, QREG_CC_OP); >> - s->cc_op = CC_OP_FLAGS; >> + if (s->cc_op == CC_OP_DYNAMIC) { >> + gen_helper_flush_flags(QREG_CC_DEST, cpu_env, QREG_CC_OP); >> + } else { >> + gen_helper_flush_flags(QREG_CC_DEST, cpu_env, >> tcg_const_i32(s->cc_op)); >> + } > > That const needs to be freed. perhaps I'm wrong, what I had understood is: tcg_const_i32() creates a tcg_temp_new_i32(), and tcg_temp_new_i32() are automatically freed at end of tcg block (whereas tcg_const_local adn tcg_temp_local are not). >> @@ -1248,7 +1294,6 @@ DISAS_INSN(bitop_im) >> DEST_EA(env, insn, opsize, tmp, &addr); >> } >> } >> - >> DISAS_INSN(arith_im) >> { >> int op; > > Careful with the errant whitespace changes. > >> @@ -1706,16 +1745,18 @@ DISAS_INSN(branch) >> /* bsr */ >> gen_push(s, tcg_const_i32(s->pc)); >> } >> - gen_flush_cc_op(s); >> if (op > 1) { >> /* Bcc */ >> l1 = gen_new_label(); >> gen_jmpcc(s, ((insn >> 8) & 0xf) ^ 1, l1); >> + update_cc_op(s); >> gen_jmp_tb(s, 1, base + offset); >> gen_set_label(l1); >> + update_cc_op(s); >> gen_jmp_tb(s, 0, s->pc); > > Ideally you'd do this only once, before the jmpcc. > > > r~