Re: [Qemu-devel] RFC: fix for random Qemu crashes

[J. Mayer]

On Fri, 2007-11-16 at 00:49 +0100, andrzej zaborowski wrote:
> Hi,
> 
> On 16/11/2007, J. Mayer <address@hidden> wrote:
> > Some may have experienced of having some Qemu builds crashing,
> > apparently at random places, but in a reproducable way.
> > I found one reason for this crashes: it appears that with the growth of
> > the op.c file, there may be cases where we could reach the inlining
> > limits of gcc. In such a case, gcc would not inline some declared
> > "inline" function but would emit a call and provide a separate function.
> > Unfortunately, this is not acceptable in op.o context as it will
> > slowdown the emulation and because the call is likely to break the
> > specific compilation rules (ie reserved registers) used while compiling
> > op.o
> > I found some workaround to avoid this behavior and I'd like to get
> > opinions about it.
> > The first idea is to change all occurences of 'inline' with
> > 'always_inline' in all headers included in op.c. It then appeared to me
> > that always_inline is not globally declared and that the definition is
> > duplicated in vl.h and exec-all.h. But it's not declared in
> > darwin-user/qemu.h and linux-user/qemu.h, which is not consistent with
> > the declaration in vl.h. Further investigations showed me that the
> > osdep.h header is the one that is actually included everywhere. Even if
> > those are more compiler than OS dependent, I decided to move the
> > definitions for glue, tostring, likely, unlikely, always_inline and
> > REGPARM to this header so they can be globally used. I also changed the
> > include orders in darwin-user/qemu.h to be sure this header will be
> > included first. This patch is attached in common_defs.diff.
> 
> I had also noticed that glue and tostring definitions were present in
> three places in qemu and it seemed wrong to me, so I'm in favour of
> this patch. I can't say much about the other patches.
> 
> After armv6/armv7 support was merged on Sunday, I had a consistent
> segfaults in the generated code and I tracked it down to cpsr_write
> function not being inlined properly because it grew in size. Changing
> the inline to always_inline helped but we decided to instead move the
> function to target-arm/helper.c. I don't know which approach is
> better, the performance hit shouldn't be notable (in case of ARM
> cpsr).

When the problem appears in rarely used or large functions, moving them
to helpers might be the proper solution. When it appears in load/store
functions (which is the case I experienced), this would not be
acceptable, as those functions are used very often and are really time
critical. Furthermore, this problem is likely to be encountered more
often as the targets features increase and then need to be solved, even
if using more helpers might be a work-around to temporary hide the
problem.

[...]

-- 
J. Mayer <address@hidden>
Never organized