[FFmpeg-devel] [PATCH 6/7] avutil/riscv/asm: add generic push/pop helpers

[Rémi Denis-Courmont]

Le 15 août 2024 15:13:57 GMT+03:00, Niklas Haas <ffmpeg at haasn.xyz> a écrit :
>On Tue, 13 Aug 2024 18:55:24 +0300 Rémi Denis-Courmont <remi at remlab.net> wrote:
>> 
>> 
>> Le 13 août 2024 17:03:35 GMT+03:00, "J. Dekker" <jdek at itanimul.li> a écrit :
>> >From: Niklas Haas <git at haasn.dev>
>> >
>> >Generic helper macros to push/pop multiple registers at once. Expands to
>> >a single `addi` plus a sequence of XLEN-sized stores/loads.
>> >---
>> > libavutil/riscv/asm.S | 37 +++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 37 insertions(+)
>> >
>> >diff --git a/libavutil/riscv/asm.S b/libavutil/riscv/asm.S
>> >index db190e99ca..3955530e4e 100644
>> >--- a/libavutil/riscv/asm.S
>> >+++ b/libavutil/riscv/asm.S
>> >@@ -288,3 +288,40 @@
>> >         .macro  count_args args:vararg
>> >         count_args_inner 0, \args
>> >         .endm
>> >+
>> >+        /**
>> >+         * Helper macro to iterate over constant sized elements in memory
>> >+         * @param op operation to perform on each element (sized load/store)
>> >+         * @param size size in bytes per element
>> >+         * @param offset starting offset of first element
>> >+         * @param addr base address to load/store
>> >+         * @param regs registers to iterate over
>> >+         */
>> >+        .macro  for_mem op, size, offset, addr, reg, regs:vararg
>> >+        .ifnb \reg
>> >+        \op     \reg, \offset(\addr)
>> >+        for_mem \op, \size, \offset + \size, \addr, \regs
>> >+        .endif
>> >+        .endm
>> >+
>> >+        /**
>> >+         * Push a variable number of registers to the stack.
>> >+         * @param n number of registers to push
>> >+         * @param regs registers to push
>> >+         */
>> >+        .macro  push regs:vararg
>> >+        count_args \regs
>> >+        addi    sp, sp, -(num_args * (__riscv_xlen >> 3))
>> >+        for_mem sx, __riscv_xlen >> 3, 0, sp, \regs
>> >+        .endm
>> 
>> This is not in line with the psABI specification for RV32 and RV64. Ditto below.
>
>Missing alignment to multiples of 16 bytes, what else?

Nothing else strictly speaking although this also breaks the frame pointer (if enabled).

That said, I am not a fan of this approach, as it necessarily introduces a data dependency on SP, which would be easily avoided with explicit code.