On 15.09.2010 09:46, Stefan Sperling wrote:
> On Fri, Sep 10, 2010 at 03:32:14PM +0200, Stefan Sperling wrote:
>> On Thu, Sep 09, 2010 at 10:58:06PM -0000, stefan2_at_apache.org wrote:
>>> Author: stefan2
>>> Date: Thu Sep 9 22:58:06 2010
>>> New Revision: 995603
>>>
>>> URL: http://svn.apache.org/viewvc?rev=995603&view=rev
>>> Log:
>>> Try really hard to help the compiler generate good code
>>> for this frequently called function.
>>>
>>> * subversion/libsvn_subr/svn_string.c
>>> (svn_stringbuf_appendbyte): reformulate
>>>
>>> Modified:
>>> subversion/branches/performance/subversion/libsvn_subr/svn_string.c
>>>
>>> Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
>>> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=995603&r1=995602&r2=995603&view=diff
>>> ==============================================================================
>>> --- subversion/branches/performance/subversion/libsvn_subr/svn_string.c (original)
>>> +++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Thu Sep 9 22:58:06 2010
>>> @@ -403,8 +403,11 @@ svn_stringbuf_appendbyte(svn_stringbuf_t
>>> apr_size_t old_len = str->len;
>>> if (str->blocksize > old_len + 1)
>>> {
>>> - str->data[old_len] = byte;
>>> - str->data[old_len+1] = '\0';
>>> + char *dest = str->data;
>>> +
>>> + dest[old_len] = byte;
>>> + dest[old_len+1] = '\0';
>>> +
>>> str->len = old_len+1;
>>> }
>>> else
>>> @@ -412,7 +415,8 @@ svn_stringbuf_appendbyte(svn_stringbuf_t
>>> /* we need to re-allocate the string buffer
>>> * -> let the more generic implementation take care of that part
>>> */
>>> - svn_stringbuf_appendbytes(str, &byte, 1);
>>> + char b = byte;
>>> + svn_stringbuf_appendbytes(str, &b, 1);
>>> }
>>> }
>>>
>>>
>> If you're expecting this function to stay written as it now,
>> you need to add a comment explaining why it is laid out the way it is.
>> Otherwise people might change it, and the resulting generated code will
>> be less optimal again.
>>
>> That said, I have my doubts whether tweaking C code in order to tune the
>> output of certain compilers in certain versions is a useful thing to do.
>> Will this still work as expected when new compiler versions are released?
>> It seems you'd rather want to fix the optimizer of the compiler.
> This has been merged into trunk now, so I'm bumping this thread to
> repeat my question above.
>
The two optimizations shown above are indeed superfluous in C, since
there are no function calls with possible side effects between creating
the temp var and using it. A good compiler will do them itself, a great
compiler will look for possible global optimizations and might do who
knows what -- might even create a specialized inline for the second case.
It would be different if the results were used after the function call,
since we cannot use "restrict" because of our standards compatibility.
But as it is ... which compiler is this supposed to tickle to generate
better code? Whether something gets passed in a register in a call
depends on the platform ABI and inlining, not on the presence of an
extra local variable (which may just get optimized out anyway).
-- Brane
Received on 2010-09-15 10:03:07 CEST