incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r663377 - in /stdcxx/branches/4.2.x: include/valarray src/valarray.cpp tests/numerics/26.class.gslice.cpp tests/regress/26.valarray.sub.stdcxx-955.cpp
Date Fri, 06 Jun 2008 19:54:16 GMT
Travis Vitek wrote:
>  
> 
> Martin Sebor wrote:
>> vitek@apache.org wrote:
>>> Author: vitek
>>> Date: Wed Jun  4 14:48:36 2008
>>> New Revision: 663377
>>>
>> [...]
>>> Modified: stdcxx/branches/4.2.x/src/valarray.cpp
>>> URL: 
>> http://svn.apache.org/viewvc/stdcxx/branches/4.2.x/src/valarray
>> .cpp?rev=663377&r1=663376&r2=663377&view=diff
>> ===============================================================
>> ===============
>>> --- stdcxx/branches/4.2.x/src/valarray.cpp (original)
>>> +++ stdcxx/branches/4.2.x/src/valarray.cpp Wed Jun  4 14:48:36 2008
>>> @@ -41,8 +41,12 @@
>>>  {
>>>      _RWSTD_SIZE_T __n = _C_length.size ();
>>>  
>>> -    while (__n && _C_r_length [__n - 1] == _C_length [__n - 1] - 1)
>>> -        --__n;
>>> +    for (/**/; __n; --__n)
>>> +    {
>> The brace should be on the line above :)
>>
> 
> Blarg!
> 
>>> +        if (   _C_length [__n - 1]
>>> +            && _C_r_length [__n - 1] != _C_length [__n - 1] - 1)
>> Also, I wonder if it might help generate more optimal code to write
>> the loop like so:
>>
>>     while (__n) {
>>         --__n;
>>
>>         if (_C_length [n] && _C_r_length [n] != _C_length [n] - 1)
>>             break;
>>     }
>>
> 
> It is possible it would help generate better code, but this code changes
> the behavior of the loop. We now decrement __n before checking the
> condition; when we break __n will be off-by-one. Obviously we can deal
> with that by incrementing __n before the break.

I should have looked at the rest of the file and not just the diff
before saying anything.

> 
> I'll take a quick look at some disassembly and decide if it is worth
> making the change or not.

It probably doesn't matter in the grand scheme of things. The new
gslice computes the indices just once and caches the result which
should be a much more significant optimization.

Martin


Mime
View raw message