Apparently, the contents of xen/include/asm-x86/string.h have been derived from Linux i386 code, with a few adjustments for x86-64, and with some memory clobbers removed from functions where the memory is only read. That latter fact is what I''m curious about: Without these clobbers (and without appropriate input constraints added), how is the compiler supposed to know that the memory pointed to by some of the other inputs is being accessed? I''m specifically running into a problem where I wanted to insert an extra strcmp() in cmdline_parse() (in xen/common/kernel.c) after the zero termination of the entire option and the call to strchr() to determine whether the option has a value specified. gcc 3.3.1 (SuSE version), which I''m using on one of my build machines, reorders things so the zero termination happens only after the newly added strcmp(), clearly because it can''t derive that strcmp() relies on this very memory being up-to-date. Surprisingly, using 3.4.4 or 4.0.3 (gnu.org version) doesn''t yield the same problem, but I''m nevertheless convinced this is not a compiler bug in 3.3.1. So there are basically two questions: 1) Why were the (questionable) inline versions from i386 Linux chosen over just using the gcc intrinsics (as x86-64 Linux does, except for a special case of memcpy())? 2) Why were the memory clobbers removed without at least replacing them with appropriate input constraints? Thanks, Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 24 May 2006, at 09:55, Jan Beulich wrote:> 1) Why were the (questionable) inline versions from i386 Linux chosen > over just using the gcc intrinsics (as x86-64 > Linux does, except for a special case of memcpy())?Intrinsics are a total pain. Sometimes the compiler inlines, sometimes it doesn''t. Sometimes it emits the __builtin_foo symbol, sometime it emits foo. Sometime when the function __builtin_foo is defined in string.c it gets the name __builtin_foo, but sometimes it gets the name foo. Getting this to work for a range of compiler versions on i386 (that''s where I see the wide range of behaviours) would be hassle. The best solution is just to remove the arch-specific definitions. None of the uses in Xen are performance critical.> 2) Why were the memory clobbers removed without at least replacing > them with appropriate input constraints?Maybe I was having a bad day. :-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>> 2) Why were the memory clobbers removed without at least replacing >> them with appropriate input constraints? > >Maybe I was having a bad day. :-)In case you still want to leave them in - here''s a patch to add the missing constraints. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>The best solution is just to remove the arch-specific definitions. None >of the uses in Xen are performance critical.I see you removed all the str* stuff. memcmp, however, suffers from the same original problem, and I would doubt it is used in performance critical code if none of the str* functions are. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 26 May 2006, at 12:14, Jan Beulich wrote:>> The best solution is just to remove the arch-specific definitions. >> None >> of the uses in Xen are performance critical. > > I see you removed all the str* stuff. memcmp, however, suffers from > the same original problem, and I would doubt it is > used in performance critical code if none of the str* functions are.We use the compiler intrinsic for memcmp, and have no inline asm version. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 26 May 2006, at 13:19, Keir Fraser wrote:>> >> I see you removed all the str* stuff. memcmp, however, suffers from >> the same original problem, and I would doubt it is >> used in performance critical code if none of the str* functions are. > > We use the compiler intrinsic for memcmp, and have no inline asm > version.Oh, ITYM memchr(). You''re absolutely right. It doesn''t have *any* users so I''ll just remove the incorrect inline asm version. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>>> Keir Fraser <Keir.Fraser@cl.cam.ac.uk> 26.05.06 14:19 >>> > >On 26 May 2006, at 12:14, Jan Beulich wrote: > >>> The best solution is just to remove the arch-specific definitions. >>> None >>> of the uses in Xen are performance critical. >> >> I see you removed all the str* stuff. memcmp, however, suffers from >> the same original problem, and I would doubt it is >> used in performance critical code if none of the str* functions are. > >We use the compiler intrinsic for memcmp, and have no inline asm >version.Sorry, I meant memchr(). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel