From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcBY9-0005TX-SN for qemu-devel@nongnu.org; Wed, 16 Sep 2015 08:10:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZcBY6-00046P-LA for qemu-devel@nongnu.org; Wed, 16 Sep 2015 08:10:57 -0400 Received: from mail.ispras.ru ([83.149.199.45]:43048) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZcBY6-00045X-9J for qemu-devel@nongnu.org; Wed, 16 Sep 2015 08:10:54 -0400 From: "Pavel Dovgaluk" References: <20150710095643.13280.88767.stgit@PASHA-ISP> <20150710095708.13280.43365.stgit@PASHA-ISP> <55CC97A4.6090009@imgtec.com> <20150817214317.GB21439@aurel32.net> <000a01d0e171$1f43de80$5dcb9b80$@Dovgaluk@ispras.ru> <55F84B46.6080606@imgtec.com> In-Reply-To: <55F84B46.6080606@imgtec.com> Date: Wed, 16 Sep 2015 15:10:53 +0300 Message-ID: <001801d0f078$bd099d10$371cd730$@Dovgaluk@ispras.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Qemu-devel] [PATCH v7 04/11] target-mips: improve exception handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Leon Alrae' , 'Aurelien Jarno' Cc: pbonzini@redhat.com, rth7680@gmail.com, qemu-devel@nongnu.org, agraf@suse.de > From: Leon Alrae [mailto:leon.alrae@imgtec.com] > On 28/08/2015 10:08, Pavel Dovgaluk wrote: > >> From: Aurelien Jarno [mailto:aurelien@aurel32.net] > >> On 2015-08-13 14:12, Leon Alrae wrote: > >>> On 10/07/2015 10:57, Pavel Dovgalyuk wrote: > >>>> @@ -2364,14 +2363,12 @@ static void gen_st_cond (DisasContext *ctx, uint32_t opc, int rt, > >>>> #if defined(TARGET_MIPS64) > >>>> case OPC_SCD: > >>>> case R6_OPC_SCD: > >>>> - save_cpu_state(ctx, 1); > >>>> op_st_scd(t1, t0, rt, ctx); > >>>> opn = "scd"; > >>>> break; > >>>> #endif > >>>> case OPC_SC: > >>>> case R6_OPC_SC: > >>>> - save_cpu_state(ctx, 1); > >>>> op_st_sc(t1, t0, rt, ctx); > >>>> opn = "sc"; > >>>> break; > >>> > >>> Wouldn't we be better off assuming that conditional stores in linux-user > >>> always take an exception (we generate fake EXCP_SC exception) and avoid > >>> retranslation? After applying these changes I observed significant impact on > >>> performance in linux-user multithreaded apps, for instance c11-atomic-exec > >>> test before the change took just 2 seconds to finish, whereas now more than 30... > >> > >> This really show the impact of retranslation and why we should avoid > >> it when not necessary. Coming back to the issue here, the fact that we > >> go through retranslation is actually due to the fact that > >> helper_raise_exception has been changed to go through retranslation. > >> > >> Given the code path between user-mode and softmmu is quite different, > >> we definitely need a different code path wrt exception and retranslation > >> for the two cases. That said if we want deterministic code execution > >> (the original purpose of this patch), I don't see how we can do without > >> forcing retranslation. Pavel, do you have an idea for that? > > > > There is only one case when we can execute without retranslation - > > when the instruction is the last instruction in translation block. > > Then we can setup PC and flags before this last instruction. > > If the exception happens, we can just break the execution. > > The drawback of this method is breaking translation blocks into > > the smaller parts. > > c11-atomic-exec.4 test execution time in linux-user: > > * no changes: > real 0m3.039s > user 0m2.976s > sys 0m1.908s > > * tb_lock + patch: > real 1m1.167s > user 0m57.240s > sys 0m36.678s > > * tb_lock + patch + SC-without-retranslation: > real 0m3.016s > user 0m2.988s > sys 0m1.848s > > I had to add tb_lock() to cpu_restore_state() in the first place, otherwise > all of my multithreaded user mode tests crash QEMU with this patch. > > SC-without-retranslation (the diff below) seems to improve the situation, > and if I understand correctly we retain deterministic code execution. > Therefore if there are no objections I'll apply this patch + SC correction > to mips-next. diff below implements exactly what I meant. Pavel Dovgalyuk > > diff --git a/target-mips/translate.c b/target-mips/translate.c > index 99b99c5..006cb96 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -2060,7 +2060,7 @@ static inline void op_st_##insn(TCGv arg1, TCGv arg2, int rt, > DisasContext *ctx) > tcg_gen_movi_tl(t0, rt | ((almask << 3) & 0x20)); \ > tcg_gen_st_tl(t0, cpu_env, offsetof(CPUMIPSState, llreg)); \ > tcg_gen_st_tl(arg1, cpu_env, offsetof(CPUMIPSState, llnewval)); \ > - gen_helper_0e0i(raise_exception, EXCP_SC); \ > + generate_exception_end(ctx, EXCP_SC); \ > gen_set_label(l2); \ > tcg_gen_movi_tl(t0, 0); \ > gen_store_gpr(t0, rt); \