[FFmpeg-devel] [PATCH 2/6] avcodec/h264: change some labels to be macro-local

[James Darnley]

On 2017-04-15 14:29, Ronald S. Bultje wrote:
> Hi,
> 
> On Fri, Apr 14, 2017 at 9:46 PM, James Darnley <jdarnley at obe.tv> wrote:
> 
>> The labels get stripped leading to (slightly) nicer disassembly from
>> objdump.
>> ---
>>  libavcodec/x86/h264_idct.asm | 24 ++++++++++++------------
>>  1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavcodec/x86/h264_idct.asm b/libavcodec/x86/h264_idct.asm
>> index 878ff02..dde40e9 100644
>> --- a/libavcodec/x86/h264_idct.asm
>> +++ b/libavcodec/x86/h264_idct.asm
>> @@ -846,7 +846,7 @@ h264_add8x4_idct_sse2:
>>  %macro add16_sse2_cycle 2
>>      movzx       r0, word [r4+%2]
>>      test        r0, r0
>> -    jz .cycle%1end
>> +    jz %%skip
> 
> 
> So I've thought about it some more. I think I'd first need to understand
> what you're doing here and why.
> 
> It seems to me that the issue you're trying to address is that when you
> look at disassembly (in e.g. a debugger or objdump), it goes from label to
> label (where function entry is also a label), and so every function-local
> label means disassembly is cut off as a block, right? (Each block then
> represents a jump target or loop or something like that.)
> 
> And you don't like that, so you're getting rid of the labels, right?

Yes.  I didn't like that because the function I was looking at had (I
think) 16 labels showing in objdump output.

Strictly speaking, I'm not getting rid of the labels but just changing
them into a format that lets STRIP strip them.  Make will run STRIP to
strip labels that begin ..@ (if configure has determined that your STRIP
supports it).

Usually I don't have a problem with labels representing a loop (or 2
nested ones) because it makes it easy to see where the code jumps back to.

> So, if all of this is correct, then I agree that the output of tools like
> debugger/objdump is irritating. In fact, it has irritated me forever in any
> codec's DSP functions. But it also seems like we're moving away from a de
> facto convention if we don't use dot-labels anymore. If we do it for
> h264_idct, we should do it everywhere (for consistency). Is that what
> people want? Maybe we should follow convention and fix objdump to include
> all dot labels in a block if a CLI option is provided?

... I don't know what to say.