PR #22640 opened by mkver URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22640 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22640.patch
There are two methods used by PROLOGUE/cglobal to align the stack if needed: a) Store the original stack in another register and align and offset the stack pointer. b) Store the original value of the stack pointer at a fixed offset from the new value of rsp. The downside of the former is that it needs another spare register, the downside of the latter is that is entails a memory store and load (instead of only a reg-reg move) and that references to the memory locations of arguments are no longer valid. The choice between the two is currently completely left to the user (by passing a positive or negative value for the desired stack size). Yet there is no good reason to ever use b) if there is a volatile register available*. This commit therefore implements to use a) automatically in this case. This affects ff_h2656_put_8tap_hv16_10_avx2 on Unix64; it also affects ff_cavs_idct8_sse2 on 32bit x86 when the stack is not guaranteed to be 16 byte aligned. In order to achieve the same effect for ff_h2656_put_8tap_hv16_10_avx2, one would have request a negative stack size on Win64 and a positive stack size on Unix64. *: Although Win64 ordinarily has 7 volatile GPRs, only six are usable for the purpose of holding the original stack pointer as rax may be needed for return values. To improve this (which also affects the case where the user explicitly requested method a)) PROLOGUE would need to know whether the function uses rax for return values. >From 417adaac421613a160db8d775e382b8d17e680d8 Mon Sep 17 00:00:00 2001 From: Andreas Rheinhardt <[email protected]> Date: Fri, 27 Mar 2026 17:07:01 +0100 Subject: [PATCH] avutil/x86/x86inc: Avoid store when aligning stack There are two methods used by PROLOGUE/cglobal to align the stack if needed: a) Store the original stack in another register and align and offset the stack pointer. b) Store the original value of the stack pointer at a fixed offset from the new value of rsp. The downside of the former is that it needs another spare register, the downside of the latter is that is entails a memory store and load (instead of only a reg-reg move) and that references to the memory locations of arguments are no longer valid. The choice between the two is currently completely left to the user (by passing a positive or negative value for the desired stack size). Yet there is no good reason to ever use b) if there is a volatile register available*. This commit therefore implements to use a) automatically in this case. This affects ff_h2656_put_8tap_hv16_10_avx2 on Unix64; it also affects ff_cavs_idct8_sse2 on 32bit x86 when the stack is not guaranteed to be 16 byte aligned. In order to achieve the same effect for ff_h2656_put_8tap_hv16_10_avx2, one would have request a negative stack size on Win64 and a positive stack size on Unix64. *: Although Win64 ordinarily has 7 volatile GPRs, only six are usable for the purpose of holding the original stack pointer as rax may be needed for return values. To improve this (which also affects the case where the user explicitly requested method a)) PROLOGUE would need to know whether the function uses rax for return values. Signed-off-by: Andreas Rheinhardt <[email protected]> --- libavutil/x86/x86inc.asm | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/libavutil/x86/x86inc.asm b/libavutil/x86/x86inc.asm index 0e80ebed43..06ff732b50 100644 --- a/libavutil/x86/x86inc.asm +++ b/libavutil/x86/x86inc.asm @@ -134,8 +134,9 @@ ; allocating the specified stack size. If the required stack alignment is ; larger than the known stack alignment the stack will be manually aligned ; and an extra register will be allocated to hold the original stack -; pointer (to not invalidate r0m etc.). To prevent the use of an extra -; register as stack pointer, request a negative stack size. +; pointer (to not invalidate r0m etc.). If r0m etc. are not used, +; request a negative stack size to avoid usage of the extra register +; if beneficial. ; %4+/%5+ = list of names to define to registers ; PROLOGUE can also be invoked by adding the same options to cglobal @@ -439,11 +440,12 @@ DECLARE_REG_TMP_SIZE 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14 ; it, i.e. in [rsp+stack_size_padded], so we can restore the ; stack in a single instruction (i.e. mov rsp, rstk or mov ; rsp, [rsp+stack_size_padded]) - %if %1 < 0 ; need to store rsp on stack + %if %1 > 0 || orig_regs_used < num_volatile_gprs - WIN64 + ; can keep rsp in rstk during whole function + %xdefine rstkm rstk + %else ; need to store rsp on stack %xdefine rstkm [rsp + stack_size + %%pad] %assign %%pad %%pad + gprsize - %else ; can keep rsp in rstk during whole function - %xdefine rstkm rstk %endif %assign stack_size_padded stack_size + ((%%pad + required_stack_alignment-1) & ~(required_stack_alignment-1)) PROBE_STACK stack_size_padded @@ -460,10 +462,12 @@ DECLARE_REG_TMP_SIZE 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14 %macro SETUP_STACK_POINTER 0-1 0 %ifnum %1 %if %1 != 0 && required_stack_alignment > STACK_ALIGNMENT - %if %1 > 0 - ; Reserve an additional register for storing the original stack pointer, but avoid using - ; eax/rax for this purpose since it can potentially get overwritten as a return value. + %if %1 > 0 || regs_used < num_volatile_gprs - WIN64 + ; Reserve an additional register for storing the original stack pointer + ; if the user requested it or if we have a free volatile register. %assign regs_used (regs_used + 1) + ; Avoid using eax/rax for this purpose since it can potentially + ; get overwritten as a return value. %if ARCH_X86_64 && regs_used == 7 %assign regs_used 8 %elif ARCH_X86_64 == 0 && regs_used == 1 @@ -497,9 +501,12 @@ DECLARE_REG 12, R15, 104 DECLARE_REG 13, R12, 112 DECLARE_REG 14, R13, 120 +%assign num_volatile_gprs 7 + %macro PROLOGUE 2-5+ 0, 0 ; #args, #regs, #xmm_regs, [stack_size,] arg_names... %assign num_args %1 %assign regs_used %2 + %assign orig_regs_used regs_used ASSERT regs_used >= num_args SETUP_STACK_POINTER %4 ASSERT regs_used <= 15 @@ -638,9 +645,12 @@ DECLARE_REG 12, R15, 56 DECLARE_REG 13, R12, 64 DECLARE_REG 14, R13, 72 +%assign num_volatile_gprs 9 + %macro PROLOGUE 2-5+ 0, 0 ; #args, #regs, #xmm_regs, [stack_size,] arg_names... %assign num_args %1 %assign regs_used %2 + %assign orig_regs_used regs_used ASSERT regs_used >= num_args SETUP_STACK_POINTER %4 ASSERT regs_used <= 15 @@ -686,6 +696,8 @@ DECLARE_REG 5, edi, 24 DECLARE_REG 6, ebp, 28 %define rsp esp +%assign num_volatile_gprs 3 + %macro DECLARE_ARG 1-* %rep %0 %define r%1m [rstk + stack_offset + 4*%1 + 4] @@ -706,6 +718,7 @@ DECLARE_ARG 7, 8, 9, 10, 11, 12, 13, 14 %if regs_used > 7 %assign regs_used 7 %endif + %assign orig_regs_used regs_used SETUP_STACK_POINTER %4 ASSERT regs_used <= 7 PUSH_IF_USED 3, 4, 5, 6 -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
