Hi, On 16/11/2007, J. Mayer <[EMAIL PROTECTED]> 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). > Giving this patch, I've been able to replace all occurence of 'inline' > with 'always_inline' in all headers included from op.c (given the > generated .d file). Some would say I'd better add a #define inline > always_inline somewhere. I personnally dislike this solution as this > kind of macro as it tends to expand recursivally (always_inline > definition contains the inline word) and this may lead to compilation > warnings or errors in some context; one could do tests using the linux > kernel headers to get convinced that it can happen. > Then, I choosed to replace 'inline' by 'always_inline', which is more > invasive but have less risks of side effects. The diff is attached in > always_inline.diff. > The last thing that helps solve the problem is to change the inlining > limits of gcc, at least to compile the op.o file.Unfortunatelly, there > is no way to disable those limits (I checked in the source code), then I > put them to an arbitrary high level. I also added the -funit-at-a-time > switch, as this kind of optimisation would not be relevant in op.o > context. The diff is attached in gcc_inline_limits.diff. > > Please comment. > > -- > J. Mayer <[EMAIL PROTECTED]> > Never organized > > Regards