| Message ID | 1470746461-26827-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 09.08.16 at 14:41, <andrew.cooper3@citrix.com> wrote: > The use of -fno-builtin inhibits this automatic transformation. Manually > tranform the callsites. This causes constructs such as strlen("literal") to > be evaluated at compile time, and certain simple operations to be replaced > with repeated string operations. Iirc this was specifically avoided in at least older Linux because of compiler issues. Are you reasonably certain that those issues predate gcc 4.1.x (and don't exist at all in clang)? Other than that I'm of course fine with this patch. Jan
On 09/08/16 15:01, Jan Beulich wrote: >>>> On 09.08.16 at 14:41, <andrew.cooper3@citrix.com> wrote: >> The use of -fno-builtin inhibits this automatic transformation. Manually >> tranform the callsites. This causes constructs such as strlen("literal") to >> be evaluated at compile time, and certain simple operations to be replaced >> with repeated string operations. > Iirc this was specifically avoided in at least older Linux because of > compiler issues. Are you reasonably certain that those issues > predate gcc 4.1.x (and don't exist at all in clang)? Other than that > I'm of course fine with this patch. Do you have any reference to these issues? I cant find any reference. I notice that Linux doesn't use these builtins, even today, but does have arch specific implementations which are more efficient than our common ones. Still, it doesn't appear to be able to do any compile-time evaluation. ~Andrew
>>> On 09.08.16 at 16:27, <andrew.cooper3@citrix.com> wrote: > On 09/08/16 15:01, Jan Beulich wrote: >>>>> On 09.08.16 at 14:41, <andrew.cooper3@citrix.com> wrote: >>> The use of -fno-builtin inhibits this automatic transformation. Manually >>> tranform the callsites. This causes constructs such as strlen("literal") to >>> be evaluated at compile time, and certain simple operations to be replaced >>> with repeated string operations. >> Iirc this was specifically avoided in at least older Linux because of >> compiler issues. Are you reasonably certain that those issues >> predate gcc 4.1.x (and don't exist at all in clang)? Other than that >> I'm of course fine with this patch. > > Do you have any reference to these issues? I cant find any reference. This was when I barley started working with Linux, i.e. in late 2.4.x or early 2.6.x times. Querying Google seems to indicate that someone reported successful use in 2.6.3. > I notice that Linux doesn't use these builtins, even today, but does > have arch specific implementations which are more efficient than our > common ones. Still, it doesn't appear to be able to do any compile-time > evaluation. Isn't it surprising that no-one has submitted Linux patches to change that? Anyway, please consider the patch ack-ed. We can always revert it in case we run into problems (and once my hardware setup will be fully functional again, I will test with a reasonably old gcc anyway). Jan
On 09/08/16 15:39, Jan Beulich wrote: >>>> On 09.08.16 at 16:27, <andrew.cooper3@citrix.com> wrote: >> On 09/08/16 15:01, Jan Beulich wrote: >>>>>> On 09.08.16 at 14:41, <andrew.cooper3@citrix.com> wrote: >>>> The use of -fno-builtin inhibits this automatic transformation. Manually >>>> tranform the callsites. This causes constructs such as strlen("literal") to >>>> be evaluated at compile time, and certain simple operations to be replaced >>>> with repeated string operations. >>> Iirc this was specifically avoided in at least older Linux because of >>> compiler issues. Are you reasonably certain that those issues >>> predate gcc 4.1.x (and don't exist at all in clang)? Other than that >>> I'm of course fine with this patch. >> Do you have any reference to these issues? I cant find any reference. > This was when I barley started working with Linux, i.e. in late 2.4.x > or early 2.6.x times. Querying Google seems to indicate that > someone reported successful use in 2.6.3. > >> I notice that Linux doesn't use these builtins, even today, but does >> have arch specific implementations which are more efficient than our >> common ones. Still, it doesn't appear to be able to do any compile-time >> evaluation. > Isn't it surprising that no-one has submitted Linux patches to > change that? Anyway, please consider the patch ack-ed. We > can always revert it in case we run into problems (and once > my hardware setup will be fully functional again, I will test with > a reasonably old gcc anyway). It turns out that in its current form, it breaks the clang build, citing string.c:45:5: error: definition of builtin function '__builtin_strcasecmp' int strcasecmp(const char *s1, const char *s2) ^ /local/xen.git/xen/include/asm/string.h:18:28: note: expanded from macro 'strcasecmp' #define strcasecmp(s1, s2) __builtin_strcasecmp(s1, s2) ^ which is something GCC clearly doesn't mind so much. I will see about reworking it. ~Andrew
diff --git a/xen/include/asm-x86/string.h b/xen/include/asm-x86/string.h index 34c5561..e194cc1 100644 --- a/xen/include/asm-x86/string.h +++ b/xen/include/asm-x86/string.h @@ -13,4 +13,12 @@ extern void *memmove(void *dest, const void *src, size_t n); #define __HAVE_ARCH_MEMSET #define memset(s,c,n) (__builtin_memset((s),(c),(n))) +#define strcmp(s1, s2) __builtin_strcmp(s1, s2) +#define strncmp(s1, s2, n) __builtin_strncmp(s1, s2, n) +#define strcasecmp(s1, s2) __builtin_strcasecmp(s1, s2) +#define strchr(s1, c) __builtin_strchr(s1, c) +#define strrchr(s1, c) __builtin_strrchr(s1, c) +#define strstr(s1, s2) __builtin_strstr(s1, s2) +#define strlen(s1) __builtin_strlen(s1) + #endif /* __X86_STRING_H__ */
The use of -fno-builtin inhibits this automatic transformation. Manually tranform the callsites. This causes constructs such as strlen("literal") to be evaluated at compile time, and certain simple operations to be replaced with repeated string operations. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/include/asm-x86/string.h | 8 ++++++++ 1 file changed, 8 insertions(+)