From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38728) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VwGKp-0000MC-2S for qemu-devel@nongnu.org; Thu, 26 Dec 2013 14:11:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VwGKj-0007xN-0H for qemu-devel@nongnu.org; Thu, 26 Dec 2013 14:11:07 -0500 Received: from mail-pb0-x233.google.com ([2607:f8b0:400e:c01::233]:57024) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VwGKi-0007x9-Nq for qemu-devel@nongnu.org; Thu, 26 Dec 2013 14:11:00 -0500 Received: by mail-pb0-f51.google.com with SMTP id up15so8427232pbc.24 for ; Thu, 26 Dec 2013 11:10:59 -0800 (PST) Sender: Richard Henderson Message-ID: <52BC7F3E.9060303@twiddle.net> Date: Thu, 26 Dec 2013 11:10:54 -0800 From: Richard Henderson MIME-Version: 1.0 References: <1385694047-6116-1-git-send-email-rth@twiddle.net> <1385694047-6116-57-git-send-email-rth@twiddle.net> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 56/60] target-i386: Tidy gen_add_A0_im List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers On 12/26/2013 10:58 AM, Peter Maydell wrote: > On 29 November 2013 03:00, Richard Henderson wrote: >> Merge gen_op_addl_A0_im and gen_op_addq_A0_im into gen_add_A0_im >> and clean up the ifdef. >> >> Replace the one remaining user of gen_op_addl_A0_im with gen_add_A0_im. >> >> Signed-off-by: Richard Henderson >> --- >> target-i386/translate.c | 27 +++++---------------------- >> 1 file changed, 5 insertions(+), 22 deletions(-) >> >> diff --git a/target-i386/translate.c b/target-i386/translate.c >> index 19cabf6..ee9d586 100644 >> --- a/target-i386/translate.c >> +++ b/target-i386/translate.c >> @@ -376,29 +376,12 @@ static inline void gen_op_mov_v_reg(TCGMemOp ot, TCGv t0, int reg) >> } >> } >> >> -static inline void gen_op_addl_A0_im(int32_t val) >> -{ >> - tcg_gen_addi_tl(cpu_A0, cpu_A0, val); >> -#ifdef TARGET_X86_64 >> - tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); >> -#endif >> -} >> - >> -#ifdef TARGET_X86_64 >> -static inline void gen_op_addq_A0_im(int64_t val) >> -{ >> - tcg_gen_addi_tl(cpu_A0, cpu_A0, val); >> -} >> -#endif >> - >> static void gen_add_A0_im(DisasContext *s, int val) >> { >> -#ifdef TARGET_X86_64 >> - if (CODE64(s)) >> - gen_op_addq_A0_im(val); >> - else >> -#endif >> - gen_op_addl_A0_im(val); >> + tcg_gen_addi_tl(cpu_A0, cpu_A0, val); >> + if (!CODE64(s)) { >> + tcg_gen_ext32u_tl(cpu_A0, cpu_A0); >> + } >> } >> >> static inline void gen_op_jmp_T0(void) >> @@ -6231,7 +6214,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, >> exception */ >> gen_op_jmp_T0(); >> /* pop selector */ >> - gen_op_addl_A0_im(1 << dflag); >> + gen_add_A0_im(s, 1 << dflag); > > Why is it OK that we no longer zero extend the result of > the add from 32 to 64 bits if CODE64(s) ? Previously we > did the extend unconditionally. I can only imagine that's a bug, to have suddenly zapped the high 32-bits of the address from which we're loading. Indeed, even this is probably not 100% correct wrt stack segment wraparound. Probably better to generate both addresses from ESP and ESP+C from scratch, using gen_lea_v_seg. r~ r~