| Message ID | 20170415014618.1592-3-jdarnley@obe.tv |
|---|---|
| State | New |
| Headers | show |
Hi, On Fri, Apr 14, 2017 at 9:46 PM, James Darnley <jdarnley@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? 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? Ronald
On 2017-04-15 14:29, Ronald S. Bultje wrote: > Hi, > > On Fri, Apr 14, 2017 at 9:46 PM, James Darnley <jdarnley@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.
What about just using strip -x on the assembly files to discard the local symbols?
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 mov r0d, dword [r1+%1*8] %if ARCH_X86_64 add r0, r5 @@ -854,7 +854,7 @@ h264_add8x4_idct_sse2: add r0, r0m %endif call h264_add8x4_idct_sse2 -.cycle%1end: +%%skip: %if %1 < 7 add r2, 64 %endif @@ -883,7 +883,7 @@ REP_RET %macro add16intra_sse2_cycle 2 movzx r0, word [r4+%2] test r0, r0 - jz .try%1dc + jz %%trydc mov r0d, dword [r1+%1*8] %if ARCH_X86_64 add r0, r7 @@ -891,11 +891,11 @@ REP_RET add r0, r0m %endif call h264_add8x4_idct_sse2 - jmp .cycle%1end -.try%1dc: + jmp %%skip +%%trydc: movsx r0, word [r2 ] or r0w, word [r2+32] - jz .cycle%1end + jz %%skip mov r0d, dword [r1+%1*8] %if ARCH_X86_64 add r0, r7 @@ -903,7 +903,7 @@ REP_RET add r0, r0m %endif call h264_idct_dc_add8_mmxext -.cycle%1end: +%%skip: %if %1 < 7 add r2, 64 %endif @@ -930,7 +930,7 @@ REP_RET %macro add8_sse2_cycle 2 movzx r0, word [r4+%2] test r0, r0 - jz .try%1dc + jz %%trydc %if ARCH_X86_64 mov r0d, dword [r1+(%1&1)*8+64*(1+(%1>>1))] add r0, [r7] @@ -940,11 +940,11 @@ REP_RET add r0, dword [r1+(%1&1)*8+64*(1+(%1>>1))] %endif call h264_add8x4_idct_sse2 - jmp .cycle%1end -.try%1dc: + jmp %%cycle_end +%%trydc: movsx r0, word [r2 ] or r0w, word [r2+32] - jz .cycle%1end + jz %%cycle_end %if ARCH_X86_64 mov r0d, dword [r1+(%1&1)*8+64*(1+(%1>>1))] add r0, [r7] @@ -954,7 +954,7 @@ REP_RET add r0, dword [r1+(%1&1)*8+64*(1+(%1>>1))] %endif call h264_idct_dc_add8_mmxext -.cycle%1end: +%%cycle_end: %if %1 == 1 add r2, 384+64 %elif %1 < 3