From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1L3fq5-0002bf-Cb for qemu-devel@nongnu.org; Fri, 21 Nov 2008 18:55:05 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1L3fq4-0002bQ-Va for qemu-devel@nongnu.org; Fri, 21 Nov 2008 18:55:05 -0500 Received: from [199.232.76.173] (port=42423 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1L3fq4-0002bN-Og for qemu-devel@nongnu.org; Fri, 21 Nov 2008 18:55:04 -0500 Received: from hall.aurel32.net ([88.191.82.174]:57452) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1L3fq4-0006hP-9h for qemu-devel@nongnu.org; Fri, 21 Nov 2008 18:55:04 -0500 Received: from volta-wlan.aurel32.net ([2002:52e8:2fb:ffff:21d:e0ff:fe49:1047] helo=volta.aurel32.net) by hall.aurel32.net with esmtpsa (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1L3fq2-0005ir-Gv for qemu-devel@nongnu.org; Sat, 22 Nov 2008 00:55:02 +0100 Received: from aurel32 by volta.aurel32.net with local (Exim 4.69) (envelope-from ) id 1L3fq1-0003Fy-Vm for qemu-devel@nongnu.org; Sat, 22 Nov 2008 00:55:01 +0100 Date: Sat, 22 Nov 2008 00:55:01 +0100 From: Aurelien Jarno Subject: Re: [Qemu-devel] Re: [PATCH] target-sh4: fix 64-bit fmov to/from memory Message-ID: <20081121235501.GD4884@volta.aurel32.net> References: <20081121214632.GQ21493@volta.aurel32.net> <1227306234-14368-1-git-send-email-mans@mansr.com> <20081121230258.GC4884@volta.aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Fri, Nov 21, 2008 at 11:30:43PM +0000, Måns Rullgård wrote: > Aurelien Jarno writes: > > > On Fri, Nov 21, 2008 at 10:23:54PM +0000, Mans Rullgard wrote: > >> When loading/storing a register pair, the even-numbered register > >> always maps to the low 32 bits of memory independently of target > >> endian configuration. > >> > >> Signed-off-by: Mans Rullgard > >> --- > >> target-sh4/translate.c | 61 ++++++++++++++++++++++++----------------------- > >> 1 files changed, 31 insertions(+), 30 deletions(-) > >> > >> diff --git a/target-sh4/translate.c b/target-sh4/translate.c > >> index 84a3f40..74894e9 100644 > >> --- a/target-sh4/translate.c > >> +++ b/target-sh4/translate.c > >> @@ -991,31 +991,35 @@ static void _decode_opc(DisasContext * ctx) > >> return; > >> case 0xf00a: /* fmov {F,D,X}Rm,@Rn - FPSCR: Nothing */ > >> if (ctx->fpscr & FPSCR_SZ) { > >> - TCGv_i64 fp = tcg_temp_new_i64(); > >> - gen_load_fpr64(fp, XREG(B7_4)); > >> - tcg_gen_qemu_st64(fp, REG(B11_8), ctx->memidx); > >> - tcg_temp_free_i64(fp); > >> + TCGv addr_hi = tcg_temp_new(); > >> + int fr = XREG(B7_4); > >> + tcg_gen_addi_i32(addr_hi, REG(B11_8), 4); > >> + tcg_gen_qemu_st32(cpu_fregs[fr ], REG(B11_8), ctx->memidx); > >> + tcg_gen_qemu_st32(cpu_fregs[fr+1], addr_hi, ctx->memidx); > >> + tcg_temp_free(addr_hi); > >> } else { > >> tcg_gen_qemu_st32(cpu_fregs[FREG(B7_4)], REG(B11_8), ctx->memidx); > >> } > >> return; > >> case 0xf008: /* fmov @Rm,{F,D,X}Rn - FPSCR: Nothing */ > >> if (ctx->fpscr & FPSCR_SZ) { > >> - TCGv_i64 fp = tcg_temp_new_i64(); > >> - tcg_gen_qemu_ld64(fp, REG(B7_4), ctx->memidx); > >> - gen_store_fpr64(fp, XREG(B11_8)); > >> - tcg_temp_free_i64(fp); > >> + TCGv addr_hi = tcg_temp_new(); > >> + int fr = XREG(B11_8); > >> + tcg_gen_addi_i32(addr_hi, REG(B7_4), 4); > >> + tcg_gen_qemu_ld32u(cpu_fregs[fr ], REG(B7_4), ctx->memidx); > >> + tcg_gen_qemu_ld32u(cpu_fregs[fr+1], addr_hi, ctx->memidx); > >> + tcg_temp_free(addr_hi); > >> } else { > >> tcg_gen_qemu_ld32u(cpu_fregs[FREG(B11_8)], REG(B7_4), ctx->memidx); > >> } > >> return; > >> case 0xf009: /* fmov @Rm+,{F,D,X}Rn - FPSCR: Nothing */ > >> if (ctx->fpscr & FPSCR_SZ) { > >> - TCGv_i64 fp = tcg_temp_new_i64(); > >> - tcg_gen_qemu_ld64(fp, REG(B7_4), ctx->memidx); > >> - gen_store_fpr64(fp, XREG(B11_8)); > >> - tcg_temp_free_i64(fp); > >> - tcg_gen_addi_i32(REG(B7_4),REG(B7_4), 8); > >> + int fr = XREG(B11_8); > >> + tcg_gen_qemu_ld32u(cpu_fregs[fr ], REG(B7_4), ctx->memidx); > >> + tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 4); > >> + tcg_gen_qemu_ld32u(cpu_fregs[fr+1], REG(B7_4), ctx->memidx); > >> + tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 4); > > > > This is wrong, the address register should only be incremented after the > > last load instruction, so that it has the correct value in case of > > exception. > > You're quite right. In fact, shouldn't the 32-bit values be loaded > into a temporary locations (at least the first to be loaded) in case > the second load generates an exception? The manual doesn't seem to > allow a partial load in such a situation, so I'd assume it's not safe. > There is nothing in the manual, but on most CPUs the value in the register is then defined as unpredictable. I don't think it is important to preserve the register value at this point. Preserving the address value is important so that the instruction could be re-executed after an exception, like a TLB miss for example. -- .''`. Aurelien Jarno | GPG: 1024D/F1BCDB73 : :' : Debian developer | Electrical Engineer `. `' aurel32@debian.org | aurelien@aurel32.net `- people.debian.org/~aurel32 | www.aurel32.net