> On 9 Oct 2022, at 16:11, Rémi Denis-Courmont <r...@remlab.net> wrote: > > Le sunnuntaina 9. lokakuuta 2022, 16.14.47 EEST Reimar Döffinger a écrit : >> Use load/store instructions that modify sp to save >> registers to stack, like it is done for all other >> functions. >> At least valgrind complains about the current code. >> --- >> libavutil/aarch64/tx_float_neon.S | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/libavutil/aarch64/tx_float_neon.S >> b/libavutil/aarch64/tx_float_neon.S index 4126c3b812..4be93cc963 100644 >> --- a/libavutil/aarch64/tx_float_neon.S >> +++ b/libavutil/aarch64/tx_float_neon.S >> @@ -866,10 +866,10 @@ FFT16_FN ns_float, 1 >> >> .macro FFT32_FN name, no_perm >> function ff_tx_fft32_\name\()_neon, export=1 >> - stp d8, d9, [sp, #-16] >> - stp d10, d11, [sp, #-32] >> - stp d12, d13, [sp, #-48] >> - stp d14, d15, [sp, #-64] >> + stp d8, d9, [sp, #-16]! >> + stp d10, d11, [sp, #-16]! >> + stp d12, d13, [sp, #-16]! >> + stp d14, d15, [sp, #-16]! > > While this fixes the ABI violation, it introduces multiple data dependencies > on > stack pointer due to write-back.
That is true in principle, this is not done consistently at all. > The idiomatic way to do this is to allocate the entire needed stack space in > the first store / last load, and use positive offsets elsewhence. Are you sure this is really relevant at all, considering it's so rarely done in the code? I'm fine looking into doing that consistently if that's what it takes, but I don't think it's good to make this the only function in the file (if not the whole lib) that does it this way. If we want the idiomatic way to be used, we should update all the code so people get the right idea. (I'll note that some places might be tricky, there seems to be code that updates sp back and forth in a loop which looks like it might be for no reason, or there is a weird reason - check SR_TRANSFORM_DEF in the same file). Just to clarify the details, then end result should look like this? - stp d8, d9, [sp, #-16] - stp d10, d11, [sp, #-32] - stp d12, d13, [sp, #-48] - stp d14, d15, [sp, #-64] + stp d14, d15, [sp, #-16*4]! + stp d8, d9, [sp, #16*3] + stp d10, d11, [sp, #16*2] + stp d12, d13, [sp, #16] Or something slightly different? Best regards, Reimar _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
