From: "Pavel Dovgaluk" <Pavel.Dovgaluk@ispras.ru>
To: 'Aurelien Jarno' <aurelien@aurel32.net>
Cc: pbonzini@redhat.com, rth7680@gmail.com, leon.alrae@imgtec.com,
qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386
Date: Thu, 18 Jun 2015 11:58:47 +0300 [thread overview]
Message-ID: <001301d0a9a4$fcc11f70$f6435e50$@Dovgaluk@ispras.ru> (raw)
In-Reply-To: <20150618081640.GK931@aurel32.net>
> From: Aurelien Jarno [mailto:aurelien@aurel32.net]
> On 2015-06-18 10:12, Pavel Dovgaluk wrote:
> > > From: Aurelien Jarno [mailto:aurelien@aurel32.net]
> > > On 2015-06-17 15:41, Pavel Dovgalyuk wrote:
> > > > In icount mode every translation block looks as follows:
> > > >
> > > > if icount < n then exit
> > > > icount -= n
> > > > instr1
> > > > instr2
> > > > ...
> > > > instrn
> > > > exit
> > > >
> > > > When one of these instructions initiates an exception, icount should be
> > > > restored and adjusted number of instructions should be subtracted from icount
> > > > instead of initial n.
> > > >
> > > > tlb_fill function passes retaddr to raise_exception, which allows restoring
> > > > current instructions in TB and correct icount calculation.
> > > >
> > > > When exception triggered with other function (e.g. by embedding call to
> > > > exception raising helper into TB), then PC is not passed as retaddr and
> > > > correct icount is not recovered. In such cases icount will be decreased
> > > > by the value equal to the size of TB.
> > >
> > > Looking at how icount work, I see it's basically a variable in the CPU
> > > state (icount_decr.u16.low), which is already accessed from the TB.
> > > Couldn't we adjust it using additional code before generating an
> > > exception, when in icount mode.
> > >
> > > For example for MIPS, we can add some code before generate_exception
> > > which use the value from s->gen_opc_icount[j] to adjust
> > > the variable icount_decr.u16.low.
> >
> > It is possible, but it will incur additional overhead, because we will
> > have to update icount every time the exception might be generated.
> > We'll have to update icount value before and after every helper call,
> > that can cause an exception:
> >
> > icount -= n
> > ...
> > instr_k
> > icount += n - k
> > helper
> > icount -= n - k
> > ...
> >
> > And this overhead will slowdown the code even if no exception occur.
>
> That's where I might disagree. Retranslation seems a very good idea on
> the paper, but in practice it doesn't seems to always bring the
> performance improvement it should. In addition it seems to be highly
> dependent on the target. Just to give some numbers, on MIPS (as your
> patch originally concerns this architecture), 40% of code generation is
> actually due to retranslation. The problem is that over the time we have
> improved a lot the code generation (liveness analysis, better register
> allocation, constant propagation, ...) and thus we have increased the
> code generation time. While it clearly has some benefits when this code
> is actually executed, it's not the case when the code is simply
> retranslated. In short we spend more time to find the CPU state
> corresponding to an exception than before.
>
...
>
> All of that to say that I am worried for the performances to see more
> paths through the retranslation code, especially on MIPS as it seems to
> be costly. That said I haven't really look in details at other targets,
> nor hosts.
I fixed syscalls, exceptions that occur without any conditions,
and removed redundant calls to save_cpu_state. Then I measured the performance
without enabling icount. And Linux boots even faster than with original version.
I'll submit this version for review soon.
> Now to come back about your patches, we might want to simply fix icount
> first, even if it has some performance impact, and deal with the
> retranslation issue separately, as it concerns more than just icount.
Pavel Dovgalyuk
next prev parent reply other threads:[~2015-06-18 8:58 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-17 12:41 [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386 Pavel Dovgalyuk
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 1/3] softmmu: add helper function to pass through retaddr Pavel Dovgalyuk
2015-06-17 12:53 ` Paolo Bonzini
2015-06-18 5:17 ` Pavel Dovgaluk
2015-06-18 8:16 ` Paolo Bonzini
2015-06-18 8:20 ` Aurelien Jarno
2015-06-18 9:24 ` Pavel Dovgaluk
2015-06-18 9:30 ` Paolo Bonzini
2015-06-18 9:33 ` Pavel Dovgaluk
2015-06-18 9:35 ` Paolo Bonzini
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 2/3] target-mips: exceptions handling in icount mode Pavel Dovgalyuk
2015-06-17 13:05 ` Aurelien Jarno
2015-06-17 12:42 ` [Qemu-devel] [PATCH v2 3/3] target-i386: fix memory operations in helpers Pavel Dovgalyuk
2015-06-17 13:27 ` Aurelien Jarno
2015-06-17 13:24 ` [Qemu-devel] [PATCH v2 0/3] Fix exceptions handling for MIPS and i386 Aurelien Jarno
2015-06-18 6:18 ` Pavel Dovgaluk
2015-06-17 14:19 ` Aurelien Jarno
2015-06-18 7:12 ` Pavel Dovgaluk
2015-06-18 8:16 ` Aurelien Jarno
2015-06-18 8:58 ` Pavel Dovgaluk [this message]
2015-06-18 9:08 ` Aurelien Jarno
2015-06-18 9:29 ` Paolo Bonzini
2015-06-18 9:42 ` Aurelien Jarno
2015-06-18 10:02 ` Paolo Bonzini
2015-06-18 17:42 ` Aurelien Jarno
2015-06-19 5:09 ` Pavel Dovgaluk
2015-06-19 8:22 ` Aurelien Jarno
[not found] ` <55826f70.2215370a.4634.ffff91b2SMTPIN_ADDED_BROKEN@mx.google.com>
2015-06-18 7:51 ` Peter Maydell
2015-06-18 7:56 ` Pavel Dovgaluk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='001301d0a9a4$fcc11f70$f6435e50$@Dovgaluk@ispras.ru' \
--to=pavel.dovgaluk@ispras.ru \
--cc=aurelien@aurel32.net \
--cc=leon.alrae@imgtec.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth7680@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.