| Message ID | 20240813140338.143045-6-jdek@itanimul.li |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/7] checkasm: add csv/tsv bench output | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
Le 13 août 2024 17:03:35 GMT+03:00, "J. Dekker" <jdek@itanimul.li> a écrit : >From: Niklas Haas <git@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. It's also not in line with the RV128 ABI since that doesn't even exist yet. >+ >+ /** >+ * Pop a variable number of registers from the stack. >+ * @param n number of registers to pop >+ * @param[out] regs registers to pop >+ */ >+ .macro pop regs:vararg >+ count_args \regs >+ for_mem lx, __riscv_xlen >> 3, 0, sp, \regs >+ addi sp, sp, num_args * (__riscv_xlen >> 3) >+ .endm
On Tue, 13 Aug 2024 18:55:24 +0300 Rémi Denis-Courmont <remi@remlab.net> wrote: > > > Le 13 août 2024 17:03:35 GMT+03:00, "J. Dekker" <jdek@itanimul.li> a écrit : > >From: Niklas Haas <git@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? > > It's also not in line with the RV128 ABI since that doesn't even exist yet. > > >+ > >+ /** > >+ * Pop a variable number of registers from the stack. > >+ * @param n number of registers to pop > >+ * @param[out] regs registers to pop > >+ */ > >+ .macro pop regs:vararg > >+ count_args \regs > >+ for_mem lx, __riscv_xlen >> 3, 0, sp, \regs > >+ addi sp, sp, num_args * (__riscv_xlen >> 3) > >+ .endm > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Rémi Denis-Courmont <remi@remlab.net> writes: > Le 13 août 2024 17:03:35 GMT+03:00, "J. Dekker" <jdek@itanimul.li> a écrit : >>From: Niklas Haas <git@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. > > It's also not in line with the RV128 ABI since that doesn't even exist yet. Why is it not in line with the psABI specification? (other than the lack of 16byte alignment as mentioned by Niklas).
Le 15 août 2024 15:13:57 GMT+03:00, Niklas Haas <ffmpeg@haasn.xyz> a écrit : >On Tue, 13 Aug 2024 18:55:24 +0300 Rémi Denis-Courmont <remi@remlab.net> wrote: >> >> >> Le 13 août 2024 17:03:35 GMT+03:00, "J. Dekker" <jdek@itanimul.li> a écrit : >> >From: Niklas Haas <git@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.
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 + + /** + * Pop a variable number of registers from the stack. + * @param n number of registers to pop + * @param[out] regs registers to pop + */ + .macro pop regs:vararg + count_args \regs + for_mem lx, __riscv_xlen >> 3, 0, sp, \regs + addi sp, sp, num_args * (__riscv_xlen >> 3) + .endm
From: Niklas Haas <git@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(+)