diff mbox series

[FFmpeg-devel,3/3] x86/vf_blend: fix warnings about trailing empty parameters

Message ID 20200709145325.7812-3-jamrial@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/3] avutil/x86inc: fix warnings when assembling with Nasm 2.15 | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

James Almer July 9, 2020, 2:53 p.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavfilter/x86/vf_blend.asm | 90 ++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

Comments

Henrik Gramner July 10, 2020, 9:54 p.m. UTC | #1 
On Thu, Jul 9, 2020 at 4:54 PM James Almer <jamrial@gmail.com> wrote:
> @@ -38,7 +38,7 @@ pb_255: times 16 db 255
>
>  SECTION .text
>
> -%macro BLEND_INIT 2-3
> +%macro BLEND_INIT 2
>  %if ARCH_X86_64
>  cglobal blend_%1, 6, 9, %2, top, top_linesize, bottom, bottom_linesize, dst, dst_linesize, width, end, x
>      mov    widthd, dword widthm

Wont this change the output? width was previously doubled a few lines
down when called with three args (%0 means "number of arguments").

The existing macro is written in a somewhat obfuscated way using the
number of arguments as an implicit boolean flag, instead of just using
a boolean flag directly.

Maybe it'd be better to change this macro to have 3 fixed arguments
and use %3 instead of %0 == 3 in combination with adding a default 0
value in the other macros with a variable number of arguments, e.g.
"%macro BLEND_SIMPLE 2-3 0"
James Almer July 11, 2020, 1:15 p.m. UTC | #2 
On 7/10/2020 6:54 PM, Henrik Gramner wrote:
> On Thu, Jul 9, 2020 at 4:54 PM James Almer <jamrial@gmail.com> wrote:
>> @@ -38,7 +38,7 @@ pb_255: times 16 db 255
>>
>>  SECTION .text
>>
>> -%macro BLEND_INIT 2-3
>> +%macro BLEND_INIT 2
>>  %if ARCH_X86_64
>>  cglobal blend_%1, 6, 9, %2, top, top_linesize, bottom, bottom_linesize, dst, dst_linesize, width, end, x
>>      mov    widthd, dword widthm
> 
> Wont this change the output? width was previously doubled a few lines
> down when called with three args (%0 means "number of arguments").
> 
> The existing macro is written in a somewhat obfuscated way using the
> number of arguments as an implicit boolean flag, instead of just using
> a boolean flag directly.
> 
> Maybe it'd be better to change this macro to have 3 fixed arguments
> and use %3 instead of %0 == 3 in combination with adding a default 0
> value in the other macros with a variable number of arguments, e.g.
> "%macro BLEND_SIMPLE 2-3 0"

You're right, i had missed the %0 checks. I'm surprised the checkasm
test didn't break after this, seeing it does seemingly test 16bit.

Will send a new version that generates the same output.
diff mbox series

Patch

diff --git a/libavfilter/x86/vf_blend.asm b/libavfilter/x86/vf_blend.asm
index 251bbb5a12..77a6ed04c7 100644
--- a/libavfilter/x86/vf_blend.asm
+++ b/libavfilter/x86/vf_blend.asm
@@ -38,7 +38,7 @@  pb_255: times 16 db 255
 
 SECTION .text
 
-%macro BLEND_INIT 2-3
+%macro BLEND_INIT 2
 %if ARCH_X86_64
 cglobal blend_%1, 6, 9, %2, top, top_linesize, bottom, bottom_linesize, dst, dst_linesize, width, end, x
     mov    widthd, dword widthm
@@ -66,8 +66,8 @@  cglobal blend_%1, 5, 7, %2, top, top_linesize, bottom, bottom_linesize, dst, end
 REP_RET
 %endmacro
 
-%macro BLEND_SIMPLE 2-3
-BLEND_INIT %1, 2, %3
+%macro BLEND_SIMPLE 2
+BLEND_INIT %1, 2
 .nextrow:
     mov        xq, widthq
 
@@ -82,8 +82,8 @@  BLEND_END
 %endmacro
 
 ; %1 name , %2 src (b or w), %3 inter (w or d), %4 (1 if 16bit, not set if 8 bit)
-%macro GRAINEXTRACT 3-4
-BLEND_INIT %1, 6, %4
+%macro GRAINEXTRACT 3
+BLEND_INIT %1, 6
     pxor           m4, m4
 %if %0 == 4 ; 16 bit
     VBROADCASTI128 m5, [pd_32768]
@@ -182,8 +182,8 @@  BLEND_END
 %endmacro
 
 ;%1 name, %2 (b or w), %3 (set if 16 bit)
-%macro AVERAGE 2-3
-BLEND_INIT %1, 3, %3
+%macro AVERAGE 2
+BLEND_INIT %1, 3
     pcmpeqb        m2, m2
 
 .nextrow:
@@ -203,8 +203,8 @@  BLEND_END
 %endmacro
 
 ; %1 name , %2 src (b or w), %3 inter (w or d), %4 (1 if 16bit, not set if 8 bit)
-%macro GRAINMERGE 3-4
-BLEND_INIT %1, 6, %4
+%macro GRAINMERGE 3
+BLEND_INIT %1, 6
     pxor       m4, m4
 %if %0 == 4 ; 16 bit
     VBROADCASTI128       m5, [pd_32768]
@@ -288,9 +288,9 @@  BLEND_INIT divide, 4
 BLEND_END
 %endmacro
 
-%macro PHOENIX 2-3
+%macro PHOENIX 2
 ; %1 name, %2 b or w, %3 (opt) 1 if 16 bit
-BLEND_INIT %1, 4, %3
+BLEND_INIT %1, 4
     VBROADCASTI128       m3, [pb_255]
 .nextrow:
     mov        xq, widthq
@@ -311,8 +311,8 @@  BLEND_END
 %endmacro
 
 ; %1 name , %2 src (b or w), %3 inter (w or d), %4 (1 if 16bit, not set if 8 bit)
-%macro DIFFERENCE 3-4
-BLEND_INIT %1, 5, %4
+%macro DIFFERENCE 3
+BLEND_INIT %1, 5
     pxor       m2, m2
 .nextrow:
     mov        xq, widthq
@@ -340,8 +340,8 @@  BLEND_END
 %endmacro
 
 ; %1 name , %2 src (b or w), %3 inter (w or d), %4 (1 if 16bit, not set if 8 bit)
-%macro EXTREMITY 3-4
-BLEND_INIT %1, 8, %4
+%macro EXTREMITY 3
+BLEND_INIT %1, 8
     pxor       m2, m2
 %if %0 == 4; 16 bit
     VBROADCASTI128       m4, [pd_65535]
@@ -375,8 +375,8 @@  BLEND_INIT %1, 8, %4
 BLEND_END
 %endmacro
 
-%macro NEGATION 3-4
-BLEND_INIT %1, 8, %4
+%macro NEGATION 3
+BLEND_INIT %1, 8
     pxor       m2, m2
 %if %0 == 4; 16 bit
     VBROADCASTI128       m4, [pd_65535]
@@ -433,12 +433,12 @@  EXTREMITY extremity, b, w
 NEGATION negation, b, w
 
 %if ARCH_X86_64
-BLEND_SIMPLE addition_16, addusw, 1
-BLEND_SIMPLE and_16,      and,    1
-BLEND_SIMPLE or_16,       or,     1
-BLEND_SIMPLE subtract_16, subusw, 1
-BLEND_SIMPLE xor_16,      xor,    1
-AVERAGE      average_16,  w,      1
+BLEND_SIMPLE addition_16, addusw
+BLEND_SIMPLE and_16,      and
+BLEND_SIMPLE or_16,       or
+BLEND_SIMPLE subtract_16, subusw
+BLEND_SIMPLE xor_16,      xor
+AVERAGE      average_16,  w
 %endif
 
 INIT_XMM ssse3
@@ -448,14 +448,14 @@  NEGATION negation, b, w
 
 INIT_XMM sse4
 %if ARCH_X86_64
-BLEND_SIMPLE darken_16,   minuw, 1
-BLEND_SIMPLE lighten_16,  maxuw, 1
-GRAINEXTRACT grainextract_16, w, d, 1
-GRAINMERGE   grainmerge_16, w, d, 1
-PHOENIX      phoenix_16,      w, 1
-DIFFERENCE   difference_16, w, d, 1
-EXTREMITY    extremity_16, w, d, 1
-NEGATION     negation_16, w, d, 1
+BLEND_SIMPLE darken_16,   minuw
+BLEND_SIMPLE lighten_16,  maxuw
+GRAINEXTRACT grainextract_16, w, d
+GRAINMERGE   grainmerge_16, w, d
+PHOENIX      phoenix_16,      w
+DIFFERENCE   difference_16, w, d
+EXTREMITY    extremity_16, w, d
+NEGATION     negation_16, w, d
 %endif
 
 %if HAVE_AVX2_EXTERNAL
@@ -480,19 +480,19 @@  EXTREMITY extremity, b, w
 NEGATION negation, b, w
 
 %if ARCH_X86_64
-BLEND_SIMPLE addition_16, addusw, 1
-BLEND_SIMPLE and_16,      and,    1
-BLEND_SIMPLE darken_16,   minuw,  1
-BLEND_SIMPLE lighten_16,  maxuw,  1
-BLEND_SIMPLE or_16,       or,     1
-BLEND_SIMPLE subtract_16, subusw, 1
-BLEND_SIMPLE xor_16,      xor,    1
-GRAINEXTRACT grainextract_16, w, d, 1
-AVERAGE      average_16,  w,      1
-GRAINMERGE   grainmerge_16, w, d, 1
-PHOENIX      phoenix_16,       w, 1
-DIFFERENCE   difference_16, w, d, 1
-EXTREMITY    extremity_16, w, d, 1
-NEGATION     negation_16, w, d, 1
+BLEND_SIMPLE addition_16, addusw
+BLEND_SIMPLE and_16,      and
+BLEND_SIMPLE darken_16,   minuw
+BLEND_SIMPLE lighten_16,  maxuw
+BLEND_SIMPLE or_16,       or
+BLEND_SIMPLE subtract_16, subusw
+BLEND_SIMPLE xor_16,      xor
+GRAINEXTRACT grainextract_16, w, d
+AVERAGE      average_16,  w
+GRAINMERGE   grainmerge_16, w, d
+PHOENIX      phoenix_16,       w
+DIFFERENCE   difference_16, w, d
+EXTREMITY    extremity_16, w, d
+NEGATION     negation_16, w, d
 %endif
 %endif