Luca Barbato <[email protected]> writes:
> On 8/26/11 3:33 AM, Måns Rullgård wrote:
>> "Ronald S. Bultje"<[email protected]> writes:
>>
>>> From: "Ronald S. Bultje"<[email protected]>
>>>
>>> From 52.503s (~40fps) to 27.973sec (~80fps) decoding of 480p sintel
>>> trailer, i.e. a ~2x speedup overall, on a Nexus S.
>>
>> [...]
>>
>>> --- a/libavcodec/arm/asm.S
>>> +++ b/libavcodec/arm/asm.S
>>> @@ -97,6 +97,12 @@ T add \rn, \rn, \rm
>>> T ldr \rt, [\rn]
>>> .endm
>>>
>>> +.macro ldr_dpren rt, rn, rm:vararg
>>> +A ldr \rt, [\rn, -\rm]
>>> +T sub \rt, \rn, \rm
>>> +T ldr \rt, [\rt]
>>> +.endm
>>> +
>>> .macro ldr_post rt, rn, rm:vararg
>>> A ldr \rt, [\rn], \rm
>>> T ldr \rt, [\rn]
>>> @@ -133,6 +139,12 @@ T ldrh \rt, [\rn]
>>> T add \rn, \rn, \rm
>>> .endm
>>>
>>> +.macro ldrb_post rt, rn, rm
>>> +A ldrb \rt, [\rn], \rm
>>> +T ldrb \rt, [\rn]
>>> +T add \rn, \rn, \rm
>>> +.endm
>>
>> Those macro names are very badly chosen.
>
> ldrb_post seems in line with ldr_post. ldr_dpren named as ldr_dpre
> seems equivalent to the other macros using sub. Which names you have
> in mind?
I'm still trying to figure what "dpren" is supposed to denote.
>>> +@ void vp8_luma_dc_wht(DCTELEM block[4][4][16], DCTELEM dc[16])
>>> +function ff_vp8_luma_dc_wht_armv6, export=1
>>> + push {r4 - r10, lr}
>>
>> Please use whitespace consistent with other files, ignoring old files
>> using random whitespace.
>
> which file should be used as template?
Anything written by me in the last couple of years, e.g. the vp8 files.
> which files should be reformatted?
Everything written before 2008.
> I'm having an hard time telling which should consider right. I'd use 4
> spaces, 3*14 space and 4*4 space as column. but it isn't really used
> everywhere.
I use 8 spaces initial indent to leave room for various tags (labels and
macros for conditional including) at the start of the line. Operands
start at column 24 and (mostly) use 5 positions each (so 3-char reg
names line up vertically). Instructions with more obscure operand
syntax are a bit random.
>> Here you have not even tried to schedule instructions anything
>> resembling optimally. Please read the relevant manuals an try again.
>> Same throughout file.
>
> Please mention the relevant manuals (maybe should be useful having a
> reference somewhere in the documentation). I guess you mean the
> arm-ARM tome of horror^Hwisdom.
The TRMs on http://infocenter.arm.com/ have scheduling information. As
long as you refuse to read the fine documentation, you will not be able
to write good code.
>> A bit less space before the comments would make these lines fit in 80
>> columns.
>
> using just 4 spaces indent would do?
See above.
>> I did not read the code in depth. Might do if next round looks more
>> promising.
>
> Since ronald is busy I'll try to help cleaning up.
Maybe I can allocate some work time to this. I'll ask and let you know.
--
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
