This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fixes tree-loop-distribute-patterns issues
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Adhemerval Zanella <azanella at linux dot vnet dot ibm dot com>
- Cc: "Carlos O'Donell" <carlos at redhat dot com>, "GNU C. Library" <libc-alpha at sourceware dot org>, Siddhesh Poyarekar <siddhesh at redhat dot com>
- Date: Wed, 19 Jun 2013 15:11:30 -0700 (PDT)
- Subject: Re: [PATCH] Fixes tree-loop-distribute-patterns issues
- References: <51C0AFB7 dot 1060009 at linux dot vnet dot ibm dot com> <20130618205608 dot 9CCE22C0AC at topped-with-meat dot com> <51C1BFE9 dot 4070805 at linux dot vnet dot ibm dot com> <51C1CEFC dot 9000100 at redhat dot com> <51C1FE4C dot 3020400 at linux dot vnet dot ibm dot com>
> > I'm a little worried about the fragility of the test, but if it doesn't
> > optimize to memset, then it may not optimize the internal implementation
> > to memset, and therefore we need not avoid the benefit of the optimization.
> >
> > I think this is a good approach, but we will have to see how it plays out
> > and if it is robust enough.
>
> We might make it more robust (maybe trying different loops), do you think it
> is worth to push more tests?
I had not contemplated this sort of configure check and I don't think it's
what we want at all. It doesn't matter whether the compiler actually
transforms a particular case into a call. All that matters is that the
compiler understands
__attribute__ ((optimize ("-fno-tree-loop-distribute-patterns"))).
If it does, then we'll use it every place we think it might matter.
> * include/libc-symbols.h (libc_disable_loop_to_calls): Define to
> disable loop transformation to library calls.
The log line for this is just, "New macro." Details about its purpose
belong in a comment at the definition site.
We only use the "libc_" prefix on macros when they are ones that
specifically apply to libc proper but not other contexts of the same code
(i.e. are defined differently based on NOT_IN_libc).
The name I'd favor is "inhibit_loop_to_libcall".
> * string/memmove.c (MEMMOVE): Disable loop transformation to avoid
> recursive call.
> * string/memset.c (memset): Likewise.
> * string/test-memmove.c (simple_memmove): Disable loop transformation
> to library calls.
> * string/test-memset.c (simple_memset): Likewise.
> * benchtests/bench-memmove.c (simple_memmove): Likewise.
These log entries should be concrete about what they're doing.
That is, say something like, "Define with inhibit_loop_to_libcall."
> -char *
> +libc_disable_cc_loop_to_function char *
> simple_memset (char *s, int c, size_t n)
Put the macro on its own line, after the return-type line.
That's the style we've used for internal_function in many places.
> +dnl Check wheter the compiler may replace some loop constructions
typo: "whether"
Thanks,
Roland