PR #22790 opened by Niklas Haas (haasn)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22790
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22790.patch

The original intent here was probably to make the ops code agnostic to
which operation is actually last in the list, but the existence of a
divergence between CONTINUE and FINISH already implies that we hard-code
the assumption that the final operation is a write op.

So we can just massively simplify this with a call/ret pair instead of
awkwardly exporting and then jumping back to the return label.

Signed-off-by: Niklas Haas <[email protected]>


>From b924b110981da8b0139edce2dbc202468b6c1c07 Mon Sep 17 00:00:00 2001
From: Niklas Haas <[email protected]>
Date: Sat, 11 Apr 2026 12:44:33 +0200
Subject: [PATCH] swscale/x86/ops: use plain `ret` instruction

The original intent here was probably to make the ops code agnostic to
which operation is actually last in the list, but the existence of a
divergence between CONTINUE and FINISH already implies that we hard-code
the assumption that the final operation is a write op.

So we can just massively simplify this with a call/ret pair instead of
awkwardly exporting and then jumping back to the return label.

Signed-off-by: Niklas Haas <[email protected]>
---
 libswscale/x86/ops.c          |  3 ---
 libswscale/x86/ops_common.asm | 15 +++++----------
 libswscale/x86/ops_int.asm    | 26 +++++---------------------
 3 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/libswscale/x86/ops.c b/libswscale/x86/ops.c
index ab113aa780..406d7c4c09 100644
--- a/libswscale/x86/ops.c
+++ b/libswscale/x86/ops.c
@@ -1006,9 +1006,6 @@ static int compile(SwsContext *ctx, SwsOpList *ops, 
SwsCompiledOp *out)
 #define ASSIGN_PROCESS_FUNC(NAME)                               \
     do {                                                        \
         SWS_DECL_FUNC(NAME);                                    \
-        void NAME##_return(void);                               \
-        ret = ff_sws_op_chain_append(chain, NAME##_return,      \
-                                     NULL, &(SwsOpPriv) {0});   \
         out->func = NAME;                                       \
     } while (0)
 
diff --git a/libswscale/x86/ops_common.asm b/libswscale/x86/ops_common.asm
index 0961d74a51..53e8c8c654 100644
--- a/libswscale/x86/ops_common.asm
+++ b/libswscale/x86/ops_common.asm
@@ -26,12 +26,7 @@
 ; function is responsible for the block loop, as well as initializing the
 ; plane pointers. It will jump directly into the first operation kernel,
 ; and each operation kernel will jump directly into the next one, with the
-; final kernel jumping back into the sws_process return point. (See label
-; `sws_process.return` in ops_int.asm)
-;
-; To handle the jump back to the return point, we append an extra address
-; corresponding to the correct sws_process.return label into the SwsOpChain,
-; and have the WRITE kernel jump into it as usual. (See the FINISH macro)
+; final kernel returning back into the entry point.
 ;
 ; Inside an operation chain, we use a custom calling convention to preserve
 ; registers between kernels. The exact register allocation is found further
@@ -292,15 +287,15 @@ endstruc
 %endmacro
 
 ; Final macro to end the operation chain, used by WRITE kernels to jump back
-; to the process function return point. Very similar to CONTINUE, but skips
-; incrementing the implq pointer, and also clears AVX registers to avoid
-; phantom dependencies between loop iterations.
+; to the process function. Very similar to CONTINUE, but skips incrementing
+; the implq pointer, and also clears AVX registers to avoid phantom
+; dependencies between loop iterations.
 %macro FINISH 1 ; reg
     %if vzeroupper_required
         ; we may jump back into an SSE read, so always zero upper regs here
         vzeroupper
     %endif
-    jmp %1
+    ret
     annotate_function_size
 %endmacro
 
diff --git a/libswscale/x86/ops_int.asm b/libswscale/x86/ops_int.asm
index 657a40df65..2d2d1dbc74 100644
--- a/libswscale/x86/ops_int.asm
+++ b/libswscale/x86/ops_int.asm
@@ -93,27 +93,12 @@ IF %1 > 3,  mov in3q,  [execq + SwsOpExec.in3]
 IF %1 > 1,  mov out1q, [execq + SwsOpExec.out1]
 IF %1 > 2,  mov out2q, [execq + SwsOpExec.out2]
 IF %1 > 3,  mov out3q, [execq + SwsOpExec.out3]
-            jmp [rsp] ; call into op chain
-
-; Declare a separate global label for the return point, so that we can append
-; it to the list of op function pointers from the C code, effectively ensuring
-; that we end up here again after the op chain finishes processing a line.
-; (See also: cglobal_label in x86inc.asm)
-%if FORMAT_ELF
-    global current_function %+ _return:function hidden
-%elif FORMAT_MACHO && HAVE_PRIVATE_EXTERN
-    global current_function %+ _return:private_extern
-%else
-    global current_function %+ _return
-%endif
-align function_align
-current_function %+ _return:
-
-            ; op chain always returns back here
+.loop:
+            call [rsp] ; call into op chain
             mov implq, [rsp + 8]
             inc bxd
             cmp bxd, [rsp + 20]
-            jne .continue
+            jne .loop
             ; end of line
             inc yd
             cmp yd, [rsp + 24]
@@ -131,7 +116,7 @@ IF %1 > 3,  add out3q, [execq + SwsOpExec.out_bump3]
             ; conditionally apply y bump (if non-NULL)
             mov tmp0q, [execq + SwsOpExec.in_bump_y]
             test tmp0q, tmp0q
-            jz .continue
+            jz .loop
             movsxd tmp0q, [tmp0q + yq * 4 - 4] ; load (signed) y bump
 %if %1 > 3
             mov tmp1q, tmp0q
@@ -150,8 +135,7 @@ IF %1 > 3,  add out3q, [execq + SwsOpExec.out_bump3]
 %endif
             imul tmp0q, [execq + SwsOpExec.in_stride0]
             add in0q, tmp0q
-.continue:
-            jmp [rsp]
+            jmp .loop
 .end:
             add rsp, 32
             RET
-- 
2.52.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to