[musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

To keep things simple, I just changed the instruction mnemonics appropriately,
rather than adding complexity by changing the buffer size/offsets based on ABI.

Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
---
 src/setjmp/riscv32/longjmp.S | 30 ++++++++++++++++++------------
 src/setjmp/riscv32/setjmp.S  | 30 ++++++++++++++++++------------
 src/setjmp/riscv64/longjmp.S | 30 ++++++++++++++++++------------
 src/setjmp/riscv64/setjmp.S  | 30 ++++++++++++++++++------------
 4 files changed, 72 insertions(+), 48 deletions(-)

diff --git a/src/setjmp/riscv32/longjmp.S b/src/setjmp/riscv32/longjmp.S
index f9cb3318..b4e5458d 100644
--- a/src/setjmp/riscv32/longjmp.S
+++ b/src/setjmp/riscv32/longjmp.S
@@ -23,18 +23,24 @@ longjmp:
 	lw ra,    52(a0)
 
 #ifndef __riscv_float_abi_soft
-	fld fs0,  56(a0)
-	fld fs1,  64(a0)
-	fld fs2,  72(a0)
-	fld fs3,  80(a0)
-	fld fs4,  88(a0)
-	fld fs5,  96(a0)
-	fld fs6,  104(a0)
-	fld fs7,  112(a0)
-	fld fs8,  120(a0)
-	fld fs9,  128(a0)
-	fld fs10, 136(a0)
-	fld fs11, 144(a0)
+#ifdef __riscv_float_abi_double
+#define FLX fld
+#else
+#define FLX flw
+#endif
+
+	FLX fs0,  56(a0)
+	FLX fs1,  64(a0)
+	FLX fs2,  72(a0)
+	FLX fs3,  80(a0)
+	FLX fs4,  88(a0)
+	FLX fs5,  96(a0)
+	FLX fs6,  104(a0)
+	FLX fs7,  112(a0)
+	FLX fs8,  120(a0)
+	FLX fs9,  128(a0)
+	FLX fs10, 136(a0)
+	FLX fs11, 144(a0)
 #endif
 
 	seqz a0, a1
diff --git a/src/setjmp/riscv32/setjmp.S b/src/setjmp/riscv32/setjmp.S
index 8a75cf55..5a1a41ef 100644
--- a/src/setjmp/riscv32/setjmp.S
+++ b/src/setjmp/riscv32/setjmp.S
@@ -23,18 +23,24 @@ setjmp:
 	sw ra,    52(a0)
 
 #ifndef __riscv_float_abi_soft
-	fsd fs0,  56(a0)
-	fsd fs1,  64(a0)
-	fsd fs2,  72(a0)
-	fsd fs3,  80(a0)
-	fsd fs4,  88(a0)
-	fsd fs5,  96(a0)
-	fsd fs6,  104(a0)
-	fsd fs7,  112(a0)
-	fsd fs8,  120(a0)
-	fsd fs9,  128(a0)
-	fsd fs10, 136(a0)
-	fsd fs11, 144(a0)
+#ifdef __riscv_float_abi_double
+#define FSX fsd
+#else
+#define FSX fsw
+#endif
+
+	FSX fs0,  56(a0)
+	FSX fs1,  64(a0)
+	FSX fs2,  72(a0)
+	FSX fs3,  80(a0)
+	FSX fs4,  88(a0)
+	FSX fs5,  96(a0)
+	FSX fs6,  104(a0)
+	FSX fs7,  112(a0)
+	FSX fs8,  120(a0)
+	FSX fs9,  128(a0)
+	FSX fs10, 136(a0)
+	FSX fs11, 144(a0)
 #endif
 
 	li a0, 0
diff --git a/src/setjmp/riscv64/longjmp.S b/src/setjmp/riscv64/longjmp.S
index 41e2d210..982475c7 100644
--- a/src/setjmp/riscv64/longjmp.S
+++ b/src/setjmp/riscv64/longjmp.S
@@ -23,18 +23,24 @@ longjmp:
 	ld ra,    104(a0)
 
 #ifndef __riscv_float_abi_soft
-	fld fs0,  112(a0)
-	fld fs1,  120(a0)
-	fld fs2,  128(a0)
-	fld fs3,  136(a0)
-	fld fs4,  144(a0)
-	fld fs5,  152(a0)
-	fld fs6,  160(a0)
-	fld fs7,  168(a0)
-	fld fs8,  176(a0)
-	fld fs9,  184(a0)
-	fld fs10, 192(a0)
-	fld fs11, 200(a0)
+#ifdef __riscv_float_abi_double
+#define FLX fld
+#else
+#define FLX flw
+#endif
+
+	FLX fs0,  112(a0)
+	FLX fs1,  120(a0)
+	FLX fs2,  128(a0)
+	FLX fs3,  136(a0)
+	FLX fs4,  144(a0)
+	FLX fs5,  152(a0)
+	FLX fs6,  160(a0)
+	FLX fs7,  168(a0)
+	FLX fs8,  176(a0)
+	FLX fs9,  184(a0)
+	FLX fs10, 192(a0)
+	FLX fs11, 200(a0)
 #endif
 
 	seqz a0, a1
diff --git a/src/setjmp/riscv64/setjmp.S b/src/setjmp/riscv64/setjmp.S
index 51249672..0795bf7d 100644
--- a/src/setjmp/riscv64/setjmp.S
+++ b/src/setjmp/riscv64/setjmp.S
@@ -23,18 +23,24 @@ setjmp:
 	sd ra,    104(a0)
 
 #ifndef __riscv_float_abi_soft
-	fsd fs0,  112(a0)
-	fsd fs1,  120(a0)
-	fsd fs2,  128(a0)
-	fsd fs3,  136(a0)
-	fsd fs4,  144(a0)
-	fsd fs5,  152(a0)
-	fsd fs6,  160(a0)
-	fsd fs7,  168(a0)
-	fsd fs8,  176(a0)
-	fsd fs9,  184(a0)
-	fsd fs10, 192(a0)
-	fsd fs11, 200(a0)
+#ifdef __riscv_float_abi_double
+#define FSX fsd
+#else
+#define FSX fsw
+#endif
+
+	FSX fs0,  112(a0)
+	FSX fs1,  120(a0)
+	FSX fs2,  128(a0)
+	FSX fs3,  136(a0)
+	FSX fs4,  144(a0)
+	FSX fs5,  152(a0)
+	FSX fs6,  160(a0)
+	FSX fs7,  168(a0)
+	FSX fs8,  176(a0)
+	FSX fs9,  184(a0)
+	FSX fs10, 192(a0)
+	FSX fs11, 200(a0)
 #endif
 
 	li a0, 0
-- 
2.40.1

[musl] Re: [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

On Sat, Jun 29, 2024 at 4:04 AM Alex Rønne Petersen <alex@alexrp.com> wrote:
>
> To keep things simple, I just changed the instruction mnemonics appropriately,
> rather than adding complexity by changing the buffer size/offsets based on ABI.

Ping, just in case this patch was overlooked. :)

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Szabolcs Nagy]

* Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> To keep things simple, I just changed the instruction mnemonics appropriately,
> rather than adding complexity by changing the buffer size/offsets based on ABI.
> 
> Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>

fwiw this looks good to me.

the only weirdness is that the math code uses __riscv_flen
and this code __riscv_float_abi*. i don't know if there
is semantic difference.

> ---
>  src/setjmp/riscv32/longjmp.S | 30 ++++++++++++++++++------------
>  src/setjmp/riscv32/setjmp.S  | 30 ++++++++++++++++++------------
>  src/setjmp/riscv64/longjmp.S | 30 ++++++++++++++++++------------
>  src/setjmp/riscv64/setjmp.S  | 30 ++++++++++++++++++------------
>  4 files changed, 72 insertions(+), 48 deletions(-)
> 
> diff --git a/src/setjmp/riscv32/longjmp.S b/src/setjmp/riscv32/longjmp.S
> index f9cb3318..b4e5458d 100644
> --- a/src/setjmp/riscv32/longjmp.S
> +++ b/src/setjmp/riscv32/longjmp.S
> @@ -23,18 +23,24 @@ longjmp:
>  	lw ra,    52(a0)
>  
>  #ifndef __riscv_float_abi_soft
> -	fld fs0,  56(a0)
> -	fld fs1,  64(a0)
> -	fld fs2,  72(a0)
> -	fld fs3,  80(a0)
> -	fld fs4,  88(a0)
> -	fld fs5,  96(a0)
> -	fld fs6,  104(a0)
> -	fld fs7,  112(a0)
> -	fld fs8,  120(a0)
> -	fld fs9,  128(a0)
> -	fld fs10, 136(a0)
> -	fld fs11, 144(a0)
> +#ifdef __riscv_float_abi_double
> +#define FLX fld
> +#else
> +#define FLX flw
> +#endif
> +
> +	FLX fs0,  56(a0)
> +	FLX fs1,  64(a0)
> +	FLX fs2,  72(a0)
> +	FLX fs3,  80(a0)
> +	FLX fs4,  88(a0)
> +	FLX fs5,  96(a0)
> +	FLX fs6,  104(a0)
> +	FLX fs7,  112(a0)
> +	FLX fs8,  120(a0)
> +	FLX fs9,  128(a0)
> +	FLX fs10, 136(a0)
> +	FLX fs11, 144(a0)
>  #endif
>  
>  	seqz a0, a1
> diff --git a/src/setjmp/riscv32/setjmp.S b/src/setjmp/riscv32/setjmp.S
> index 8a75cf55..5a1a41ef 100644
> --- a/src/setjmp/riscv32/setjmp.S
> +++ b/src/setjmp/riscv32/setjmp.S
> @@ -23,18 +23,24 @@ setjmp:
>  	sw ra,    52(a0)
>  
>  #ifndef __riscv_float_abi_soft
> -	fsd fs0,  56(a0)
> -	fsd fs1,  64(a0)
> -	fsd fs2,  72(a0)
> -	fsd fs3,  80(a0)
> -	fsd fs4,  88(a0)
> -	fsd fs5,  96(a0)
> -	fsd fs6,  104(a0)
> -	fsd fs7,  112(a0)
> -	fsd fs8,  120(a0)
> -	fsd fs9,  128(a0)
> -	fsd fs10, 136(a0)
> -	fsd fs11, 144(a0)
> +#ifdef __riscv_float_abi_double
> +#define FSX fsd
> +#else
> +#define FSX fsw
> +#endif
> +
> +	FSX fs0,  56(a0)
> +	FSX fs1,  64(a0)
> +	FSX fs2,  72(a0)
> +	FSX fs3,  80(a0)
> +	FSX fs4,  88(a0)
> +	FSX fs5,  96(a0)
> +	FSX fs6,  104(a0)
> +	FSX fs7,  112(a0)
> +	FSX fs8,  120(a0)
> +	FSX fs9,  128(a0)
> +	FSX fs10, 136(a0)
> +	FSX fs11, 144(a0)
>  #endif
>  
>  	li a0, 0
> diff --git a/src/setjmp/riscv64/longjmp.S b/src/setjmp/riscv64/longjmp.S
> index 41e2d210..982475c7 100644
> --- a/src/setjmp/riscv64/longjmp.S
> +++ b/src/setjmp/riscv64/longjmp.S
> @@ -23,18 +23,24 @@ longjmp:
>  	ld ra,    104(a0)
>  
>  #ifndef __riscv_float_abi_soft
> -	fld fs0,  112(a0)
> -	fld fs1,  120(a0)
> -	fld fs2,  128(a0)
> -	fld fs3,  136(a0)
> -	fld fs4,  144(a0)
> -	fld fs5,  152(a0)
> -	fld fs6,  160(a0)
> -	fld fs7,  168(a0)
> -	fld fs8,  176(a0)
> -	fld fs9,  184(a0)
> -	fld fs10, 192(a0)
> -	fld fs11, 200(a0)
> +#ifdef __riscv_float_abi_double
> +#define FLX fld
> +#else
> +#define FLX flw
> +#endif
> +
> +	FLX fs0,  112(a0)
> +	FLX fs1,  120(a0)
> +	FLX fs2,  128(a0)
> +	FLX fs3,  136(a0)
> +	FLX fs4,  144(a0)
> +	FLX fs5,  152(a0)
> +	FLX fs6,  160(a0)
> +	FLX fs7,  168(a0)
> +	FLX fs8,  176(a0)
> +	FLX fs9,  184(a0)
> +	FLX fs10, 192(a0)
> +	FLX fs11, 200(a0)
>  #endif
>  
>  	seqz a0, a1
> diff --git a/src/setjmp/riscv64/setjmp.S b/src/setjmp/riscv64/setjmp.S
> index 51249672..0795bf7d 100644
> --- a/src/setjmp/riscv64/setjmp.S
> +++ b/src/setjmp/riscv64/setjmp.S
> @@ -23,18 +23,24 @@ setjmp:
>  	sd ra,    104(a0)
>  
>  #ifndef __riscv_float_abi_soft
> -	fsd fs0,  112(a0)
> -	fsd fs1,  120(a0)
> -	fsd fs2,  128(a0)
> -	fsd fs3,  136(a0)
> -	fsd fs4,  144(a0)
> -	fsd fs5,  152(a0)
> -	fsd fs6,  160(a0)
> -	fsd fs7,  168(a0)
> -	fsd fs8,  176(a0)
> -	fsd fs9,  184(a0)
> -	fsd fs10, 192(a0)
> -	fsd fs11, 200(a0)
> +#ifdef __riscv_float_abi_double
> +#define FSX fsd
> +#else
> +#define FSX fsw
> +#endif
> +
> +	FSX fs0,  112(a0)
> +	FSX fs1,  120(a0)
> +	FSX fs2,  128(a0)
> +	FSX fs3,  136(a0)
> +	FSX fs4,  144(a0)
> +	FSX fs5,  152(a0)
> +	FSX fs6,  160(a0)
> +	FSX fs7,  168(a0)
> +	FSX fs8,  176(a0)
> +	FSX fs9,  184(a0)
> +	FSX fs10, 192(a0)
> +	FSX fs11, 200(a0)
>  #endif
>  
>  	li a0, 0
> -- 
> 2.40.1

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
>
> * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > To keep things simple, I just changed the instruction mnemonics appropriately,
> > rather than adding complexity by changing the buffer size/offsets based on ABI.
> >
> > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
>
> fwiw this looks good to me.
>
> the only weirdness is that the math code uses __riscv_flen
> and this code __riscv_float_abi*. i don't know if there
> is semantic difference.

`__riscv_flen` tells you the width of the FP registers on the target
CPU. This is semantically distinct from `__riscv_float_abi`. For
example, while it would probably be a bit silly, there's no particular
reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine.
In that case, no code needs to concern itself with the upper bits of
the FP registers.

I took a quick peek at some of the `__riscv_flen` checks in musl. They
look ok. They're checking the capabilities of the machine for the
purposes of performing a computation; they're not making ABI
decisions. In my silly example above, if I tell the compiler to do so
with `-march=rv64...d`, it would theoretically be fine for the
compiler to generate double-precision float instructions for
computations as long as values are passed/returned according to LP64F
rules.

>
> > ---
> >  src/setjmp/riscv32/longjmp.S | 30 ++++++++++++++++++------------
> >  src/setjmp/riscv32/setjmp.S  | 30 ++++++++++++++++++------------
> >  src/setjmp/riscv64/longjmp.S | 30 ++++++++++++++++++------------
> >  src/setjmp/riscv64/setjmp.S  | 30 ++++++++++++++++++------------
> >  4 files changed, 72 insertions(+), 48 deletions(-)
> >
> > diff --git a/src/setjmp/riscv32/longjmp.S b/src/setjmp/riscv32/longjmp.S
> > index f9cb3318..b4e5458d 100644
> > --- a/src/setjmp/riscv32/longjmp.S
> > +++ b/src/setjmp/riscv32/longjmp.S
> > @@ -23,18 +23,24 @@ longjmp:
> >       lw ra,    52(a0)
> >
> >  #ifndef __riscv_float_abi_soft
> > -     fld fs0,  56(a0)
> > -     fld fs1,  64(a0)
> > -     fld fs2,  72(a0)
> > -     fld fs3,  80(a0)
> > -     fld fs4,  88(a0)
> > -     fld fs5,  96(a0)
> > -     fld fs6,  104(a0)
> > -     fld fs7,  112(a0)
> > -     fld fs8,  120(a0)
> > -     fld fs9,  128(a0)
> > -     fld fs10, 136(a0)
> > -     fld fs11, 144(a0)
> > +#ifdef __riscv_float_abi_double
> > +#define FLX fld
> > +#else
> > +#define FLX flw
> > +#endif
> > +
> > +     FLX fs0,  56(a0)
> > +     FLX fs1,  64(a0)
> > +     FLX fs2,  72(a0)
> > +     FLX fs3,  80(a0)
> > +     FLX fs4,  88(a0)
> > +     FLX fs5,  96(a0)
> > +     FLX fs6,  104(a0)
> > +     FLX fs7,  112(a0)
> > +     FLX fs8,  120(a0)
> > +     FLX fs9,  128(a0)
> > +     FLX fs10, 136(a0)
> > +     FLX fs11, 144(a0)
> >  #endif
> >
> >       seqz a0, a1
> > diff --git a/src/setjmp/riscv32/setjmp.S b/src/setjmp/riscv32/setjmp.S
> > index 8a75cf55..5a1a41ef 100644
> > --- a/src/setjmp/riscv32/setjmp.S
> > +++ b/src/setjmp/riscv32/setjmp.S
> > @@ -23,18 +23,24 @@ setjmp:
> >       sw ra,    52(a0)
> >
> >  #ifndef __riscv_float_abi_soft
> > -     fsd fs0,  56(a0)
> > -     fsd fs1,  64(a0)
> > -     fsd fs2,  72(a0)
> > -     fsd fs3,  80(a0)
> > -     fsd fs4,  88(a0)
> > -     fsd fs5,  96(a0)
> > -     fsd fs6,  104(a0)
> > -     fsd fs7,  112(a0)
> > -     fsd fs8,  120(a0)
> > -     fsd fs9,  128(a0)
> > -     fsd fs10, 136(a0)
> > -     fsd fs11, 144(a0)
> > +#ifdef __riscv_float_abi_double
> > +#define FSX fsd
> > +#else
> > +#define FSX fsw
> > +#endif
> > +
> > +     FSX fs0,  56(a0)
> > +     FSX fs1,  64(a0)
> > +     FSX fs2,  72(a0)
> > +     FSX fs3,  80(a0)
> > +     FSX fs4,  88(a0)
> > +     FSX fs5,  96(a0)
> > +     FSX fs6,  104(a0)
> > +     FSX fs7,  112(a0)
> > +     FSX fs8,  120(a0)
> > +     FSX fs9,  128(a0)
> > +     FSX fs10, 136(a0)
> > +     FSX fs11, 144(a0)
> >  #endif
> >
> >       li a0, 0
> > diff --git a/src/setjmp/riscv64/longjmp.S b/src/setjmp/riscv64/longjmp.S
> > index 41e2d210..982475c7 100644
> > --- a/src/setjmp/riscv64/longjmp.S
> > +++ b/src/setjmp/riscv64/longjmp.S
> > @@ -23,18 +23,24 @@ longjmp:
> >       ld ra,    104(a0)
> >
> >  #ifndef __riscv_float_abi_soft
> > -     fld fs0,  112(a0)
> > -     fld fs1,  120(a0)
> > -     fld fs2,  128(a0)
> > -     fld fs3,  136(a0)
> > -     fld fs4,  144(a0)
> > -     fld fs5,  152(a0)
> > -     fld fs6,  160(a0)
> > -     fld fs7,  168(a0)
> > -     fld fs8,  176(a0)
> > -     fld fs9,  184(a0)
> > -     fld fs10, 192(a0)
> > -     fld fs11, 200(a0)
> > +#ifdef __riscv_float_abi_double
> > +#define FLX fld
> > +#else
> > +#define FLX flw
> > +#endif
> > +
> > +     FLX fs0,  112(a0)
> > +     FLX fs1,  120(a0)
> > +     FLX fs2,  128(a0)
> > +     FLX fs3,  136(a0)
> > +     FLX fs4,  144(a0)
> > +     FLX fs5,  152(a0)
> > +     FLX fs6,  160(a0)
> > +     FLX fs7,  168(a0)
> > +     FLX fs8,  176(a0)
> > +     FLX fs9,  184(a0)
> > +     FLX fs10, 192(a0)
> > +     FLX fs11, 200(a0)
> >  #endif
> >
> >       seqz a0, a1
> > diff --git a/src/setjmp/riscv64/setjmp.S b/src/setjmp/riscv64/setjmp.S
> > index 51249672..0795bf7d 100644
> > --- a/src/setjmp/riscv64/setjmp.S
> > +++ b/src/setjmp/riscv64/setjmp.S
> > @@ -23,18 +23,24 @@ setjmp:
> >       sd ra,    104(a0)
> >
> >  #ifndef __riscv_float_abi_soft
> > -     fsd fs0,  112(a0)
> > -     fsd fs1,  120(a0)
> > -     fsd fs2,  128(a0)
> > -     fsd fs3,  136(a0)
> > -     fsd fs4,  144(a0)
> > -     fsd fs5,  152(a0)
> > -     fsd fs6,  160(a0)
> > -     fsd fs7,  168(a0)
> > -     fsd fs8,  176(a0)
> > -     fsd fs9,  184(a0)
> > -     fsd fs10, 192(a0)
> > -     fsd fs11, 200(a0)
> > +#ifdef __riscv_float_abi_double
> > +#define FSX fsd
> > +#else
> > +#define FSX fsw
> > +#endif
> > +
> > +     FSX fs0,  112(a0)
> > +     FSX fs1,  120(a0)
> > +     FSX fs2,  128(a0)
> > +     FSX fs3,  136(a0)
> > +     FSX fs4,  144(a0)
> > +     FSX fs5,  152(a0)
> > +     FSX fs6,  160(a0)
> > +     FSX fs7,  168(a0)
> > +     FSX fs8,  176(a0)
> > +     FSX fs9,  184(a0)
> > +     FSX fs10, 192(a0)
> > +     FSX fs11, 200(a0)
> >  #endif
> >
> >       li a0, 0
> > --
> > 2.40.1

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Rich Felker]

On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> >
> > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > >
> > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> >
> > fwiw this looks good to me.
> >
> > the only weirdness is that the math code uses __riscv_flen
> > and this code __riscv_float_abi*. i don't know if there
> > is semantic difference.
> 
> `__riscv_flen` tells you the width of the FP registers on the target
> CPU. This is semantically distinct from `__riscv_float_abi`. For
> example, while it would probably be a bit silly, there's no particular
> reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine.
> In that case, no code needs to concern itself with the upper bits of
> the FP registers.
> 
> I took a quick peek at some of the `__riscv_flen` checks in musl. They
> look ok. They're checking the capabilities of the machine for the
> purposes of performing a computation; they're not making ABI
> decisions. In my silly example above, if I tell the compiler to do so
> with `-march=rv64...d`, it would theoretically be fine for the
> compiler to generate double-precision float instructions for
> computations as long as values are passed/returned according to LP64F
> rules.

If you're building code for -sf or -sp ABI, but could run on a machine
with larger floating point register file, it's possible that the user
could have libc built not to use fp registers at all or only 32-bit
registers (respectively), but the calling application could be built
for and running on a machine with 64-bit registers. In this case we
need to understand what the ABI says. Are the 64-bit register, if
present, call-saved in lower ABI levels where they don't participate
in the calling convention? If so, no #ifdef is sufficient and there
must be a runtime hwcap check here to determine which form of
save/restore to do, like on arm and powerpc.

Rich

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

On Wed, Jul 24, 2024 at 12:58 AM Rich Felker <dalias@libc.org> wrote:
>
> On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> > On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> > >
> > > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > > >
> > > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> > >
> > > fwiw this looks good to me.
> > >
> > > the only weirdness is that the math code uses __riscv_flen
> > > and this code __riscv_float_abi*. i don't know if there
> > > is semantic difference.
> >
> > `__riscv_flen` tells you the width of the FP registers on the target
> > CPU. This is semantically distinct from `__riscv_float_abi`. For
> > example, while it would probably be a bit silly, there's no particular
> > reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine.
> > In that case, no code needs to concern itself with the upper bits of
> > the FP registers.
> >
> > I took a quick peek at some of the `__riscv_flen` checks in musl. They
> > look ok. They're checking the capabilities of the machine for the
> > purposes of performing a computation; they're not making ABI
> > decisions. In my silly example above, if I tell the compiler to do so
> > with `-march=rv64...d`, it would theoretically be fine for the
> > compiler to generate double-precision float instructions for
> > computations as long as values are passed/returned according to LP64F
> > rules.
>
> If you're building code for -sf or -sp ABI, but could run on a machine
> with larger floating point register file, it's possible that the user
> could have libc built not to use fp registers at all or only 32-bit
> registers (respectively), but the calling application could be built
> for and running on a machine with 64-bit registers. In this case we
> need to understand what the ABI says. Are the 64-bit register, if
> present, call-saved in lower ABI levels where they don't participate
> in the calling convention? If so, no #ifdef is sufficient and there
> must be a runtime hwcap check here to determine which form of
> save/restore to do, like on arm and powerpc.

I don't think this is a scenario that the ABI considers to be
supported. If you try to link code of different ABIs, you will get
linker errors such as:

    /tmp/ccz2Y86f.o: can't link soft-float modules with double-float modules

or

    /tmp/cc7rTh7R.o: can't link single-float modules with double-float modules

(The direction does not matter.)

That said, if you think we should still try to do the right thing in
this scenario, I can read through the calling convention rules and try
to figure it out.

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Rich Felker]

On Wed, Jul 24, 2024 at 01:12:33AM +0200, Alex Rønne Petersen wrote:
> On Wed, Jul 24, 2024 at 12:58 AM Rich Felker <dalias@libc.org> wrote:
> >
> > On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> > > On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> > > >
> > > > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > > > >
> > > > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> > > >
> > > > fwiw this looks good to me.
> > > >
> > > > the only weirdness is that the math code uses __riscv_flen
> > > > and this code __riscv_float_abi*. i don't know if there
> > > > is semantic difference.
> > >
> > > `__riscv_flen` tells you the width of the FP registers on the target
> > > CPU. This is semantically distinct from `__riscv_float_abi`. For
> > > example, while it would probably be a bit silly, there's no particular
> > > reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine.
> > > In that case, no code needs to concern itself with the upper bits of
> > > the FP registers.
> > >
> > > I took a quick peek at some of the `__riscv_flen` checks in musl. They
> > > look ok. They're checking the capabilities of the machine for the
> > > purposes of performing a computation; they're not making ABI
> > > decisions. In my silly example above, if I tell the compiler to do so
> > > with `-march=rv64...d`, it would theoretically be fine for the
> > > compiler to generate double-precision float instructions for
> > > computations as long as values are passed/returned according to LP64F
> > > rules.
> >
> > If you're building code for -sf or -sp ABI, but could run on a machine
> > with larger floating point register file, it's possible that the user
> > could have libc built not to use fp registers at all or only 32-bit
> > registers (respectively), but the calling application could be built
> > for and running on a machine with 64-bit registers. In this case we
> > need to understand what the ABI says. Are the 64-bit register, if
> > present, call-saved in lower ABI levels where they don't participate
> > in the calling convention? If so, no #ifdef is sufficient and there
> > must be a runtime hwcap check here to determine which form of
> > save/restore to do, like on arm and powerpc.
> 
> I don't think this is a scenario that the ABI considers to be
> supported. If you try to link code of different ABIs, you will get
> linker errors such as:
> 
>     /tmp/ccz2Y86f.o: can't link soft-float modules with double-float modules
> 
> or
> 
>     /tmp/cc7rTh7R.o: can't link single-float modules with double-float modules

Those are different ABIs. You can't link modules with mismatched ABI,
but you should be able to link modules that are both using -sf ABI (or
both using -sp ABI), where one is not using the fpu and the other is
using the full double fpu but only passing args in GPRs to conform
with the ABI. If that's not allowed, I would consider it a tooling
bug; there's no compatibility-constraint reason it can't be allowed.

Rich

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

On Wed, Jul 24, 2024 at 1:22 AM Rich Felker <dalias@libc.org> wrote:
>
> On Wed, Jul 24, 2024 at 01:12:33AM +0200, Alex Rønne Petersen wrote:
> > On Wed, Jul 24, 2024 at 12:58 AM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> > > > On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> > > > >
> > > > > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > > > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > > > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > > > > >
> > > > > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> > > > >
> > > > > fwiw this looks good to me.
> > > > >
> > > > > the only weirdness is that the math code uses __riscv_flen
> > > > > and this code __riscv_float_abi*. i don't know if there
> > > > > is semantic difference.
> > > >
> > > > `__riscv_flen` tells you the width of the FP registers on the target
> > > > CPU. This is semantically distinct from `__riscv_float_abi`. For
> > > > example, while it would probably be a bit silly, there's no particular
> > > > reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine.
> > > > In that case, no code needs to concern itself with the upper bits of
> > > > the FP registers.
> > > >
> > > > I took a quick peek at some of the `__riscv_flen` checks in musl. They
> > > > look ok. They're checking the capabilities of the machine for the
> > > > purposes of performing a computation; they're not making ABI
> > > > decisions. In my silly example above, if I tell the compiler to do so
> > > > with `-march=rv64...d`, it would theoretically be fine for the
> > > > compiler to generate double-precision float instructions for
> > > > computations as long as values are passed/returned according to LP64F
> > > > rules.
> > >
> > > If you're building code for -sf or -sp ABI, but could run on a machine
> > > with larger floating point register file, it's possible that the user
> > > could have libc built not to use fp registers at all or only 32-bit
> > > registers (respectively), but the calling application could be built
> > > for and running on a machine with 64-bit registers. In this case we
> > > need to understand what the ABI says. Are the 64-bit register, if
> > > present, call-saved in lower ABI levels where they don't participate
> > > in the calling convention? If so, no #ifdef is sufficient and there
> > > must be a runtime hwcap check here to determine which form of
> > > save/restore to do, like on arm and powerpc.
> >
> > I don't think this is a scenario that the ABI considers to be
> > supported. If you try to link code of different ABIs, you will get
> > linker errors such as:
> >
> >     /tmp/ccz2Y86f.o: can't link soft-float modules with double-float modules
> >
> > or
> >
> >     /tmp/cc7rTh7R.o: can't link single-float modules with double-float modules
>
> Those are different ABIs. You can't link modules with mismatched ABI,
> but you should be able to link modules that are both using -sf ABI (or
> both using -sp ABI), where one is not using the fpu and the other is
> using the full double fpu but only passing args in GPRs to conform
> with the ABI. If that's not allowed, I would consider it a tooling
> bug; there's no compatibility-constraint reason it can't be allowed.

Oh, of course. I misread your question, sorry.

Here's, I think, the relevant section of the calling convention:
https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#floating-point-register-convention

This part is a bit awkward (to me, at least):

    Floating-point values in callee-saved registers are only preserved
across calls if they are no larger than the width of a floating-point
register in the targeted ABI. Therefore, these registers can always be
considered temporaries if targeting the base integer calling
convention.

I'm not really sure why they're talking about "values" there; I would
think the register width (in the machine vs the ABI) is the only thing
we're concerned about in this context. I'm assuming that what they
mean is:

    Floating-point registers in the callee-saved set are only
preserved across calls if they are no larger than the width of a
floating-point register in the targeted ABI.

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Rich Felker]

On Wed, Jul 24, 2024 at 02:09:01AM +0200, Alex Rønne Petersen wrote:
> On Wed, Jul 24, 2024 at 1:22 AM Rich Felker <dalias@libc.org> wrote:
> >
> > On Wed, Jul 24, 2024 at 01:12:33AM +0200, Alex Rønne Petersen wrote:
> > > On Wed, Jul 24, 2024 at 12:58 AM Rich Felker <dalias@libc.org> wrote:
> > > >
> > > > On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> > > > > On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> > > > > >
> > > > > > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > > > > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > > > > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > > > > > >
> > > > > > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> > > > > >
> > > > > > fwiw this looks good to me.
> > > > > >
> > > > > > the only weirdness is that the math code uses __riscv_flen
> > > > > > and this code __riscv_float_abi*. i don't know if there
> > > > > > is semantic difference.
> > > > >
> > > > > `__riscv_flen` tells you the width of the FP registers on the target
> > > > > CPU. This is semantically distinct from `__riscv_float_abi`. For
> > > > > example, while it would probably be a bit silly, there's no particular
> > > > > reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine..
> > > > > In that case, no code needs to concern itself with the upper bits of
> > > > > the FP registers.
> > > > >
> > > > > I took a quick peek at some of the `__riscv_flen` checks in musl. They
> > > > > look ok. They're checking the capabilities of the machine for the
> > > > > purposes of performing a computation; they're not making ABI
> > > > > decisions. In my silly example above, if I tell the compiler to do so
> > > > > with `-march=rv64...d`, it would theoretically be fine for the
> > > > > compiler to generate double-precision float instructions for
> > > > > computations as long as values are passed/returned according to LP64F
> > > > > rules.
> > > >
> > > > If you're building code for -sf or -sp ABI, but could run on a machine
> > > > with larger floating point register file, it's possible that the user
> > > > could have libc built not to use fp registers at all or only 32-bit
> > > > registers (respectively), but the calling application could be built
> > > > for and running on a machine with 64-bit registers. In this case we
> > > > need to understand what the ABI says. Are the 64-bit register, if
> > > > present, call-saved in lower ABI levels where they don't participate
> > > > in the calling convention? If so, no #ifdef is sufficient and there
> > > > must be a runtime hwcap check here to determine which form of
> > > > save/restore to do, like on arm and powerpc.
> > >
> > > I don't think this is a scenario that the ABI considers to be
> > > supported. If you try to link code of different ABIs, you will get
> > > linker errors such as:
> > >
> > >     /tmp/ccz2Y86f.o: can't link soft-float modules with double-float modules
> > >
> > > or
> > >
> > >     /tmp/cc7rTh7R.o: can't link single-float modules with double-float modules
> >
> > Those are different ABIs. You can't link modules with mismatched ABI,
> > but you should be able to link modules that are both using -sf ABI (or
> > both using -sp ABI), where one is not using the fpu and the other is
> > using the full double fpu but only passing args in GPRs to conform
> > with the ABI. If that's not allowed, I would consider it a tooling
> > bug; there's no compatibility-constraint reason it can't be allowed.
> 
> Oh, of course. I misread your question, sorry.
> 
> Here's, I think, the relevant section of the calling convention:
> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#floating-point-register-convention
> 
> This part is a bit awkward (to me, at least):
> 
>     Floating-point values in callee-saved registers are only preserved
> across calls if they are no larger than the width of a floating-point
> register in the targeted ABI. Therefore, these registers can always be
> considered temporaries if targeting the base integer calling
> convention.
> 
> I'm not really sure why they're talking about "values" there; I would
> think the register width (in the machine vs the ABI) is the only thing
> we're concerned about in this context. I'm assuming that what they
> mean is:
> 
>     Floating-point registers in the callee-saved set are only
> preserved across calls if they are no larger than the width of a
> floating-point register in the targeted ABI.

OK, perfect. That means we only need to decide what to save based on
the ABI, not dynamic hwcap or FPU capabilities.

Rich

Re: [musl] [PATCH] riscv: Fix setjmp assembly when compiling for ilp32f/lp64f.

[Alex Rønne Petersen]

On Wed, Jul 24, 2024 at 2:13 AM Rich Felker <dalias@libc.org> wrote:
>
> On Wed, Jul 24, 2024 at 02:09:01AM +0200, Alex Rønne Petersen wrote:
> > On Wed, Jul 24, 2024 at 1:22 AM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Wed, Jul 24, 2024 at 01:12:33AM +0200, Alex Rønne Petersen wrote:
> > > > On Wed, Jul 24, 2024 at 12:58 AM Rich Felker <dalias@libc.org> wrote:
> > > > >
> > > > > On Wed, Jul 24, 2024 at 12:47:14AM +0200, Alex Rønne Petersen wrote:
> > > > > > On Tue, Jul 23, 2024 at 11:22 PM Szabolcs Nagy <nsz@port70.net> wrote:
> > > > > > >
> > > > > > > * Alex Rønne Petersen <alex@alexrp.com> [2024-06-29 04:04:34 +0200]:
> > > > > > > > To keep things simple, I just changed the instruction mnemonics appropriately,
> > > > > > > > rather than adding complexity by changing the buffer size/offsets based on ABI.
> > > > > > > >
> > > > > > > > Signed-off-by: Alex Rønne Petersen <alex@alexrp.com>
> > > > > > >
> > > > > > > fwiw this looks good to me.
> > > > > > >
> > > > > > > the only weirdness is that the math code uses __riscv_flen
> > > > > > > and this code __riscv_float_abi*. i don't know if there
> > > > > > > is semantic difference.
> > > > > >
> > > > > > `__riscv_flen` tells you the width of the FP registers on the target
> > > > > > CPU. This is semantically distinct from `__riscv_float_abi`. For
> > > > > > example, while it would probably be a bit silly, there's no particular
> > > > > > reason why I couldn't target the LP64F ABI on an RV64IMAFDC machine..
> > > > > > In that case, no code needs to concern itself with the upper bits of
> > > > > > the FP registers.
> > > > > >
> > > > > > I took a quick peek at some of the `__riscv_flen` checks in musl. They
> > > > > > look ok. They're checking the capabilities of the machine for the
> > > > > > purposes of performing a computation; they're not making ABI
> > > > > > decisions. In my silly example above, if I tell the compiler to do so
> > > > > > with `-march=rv64...d`, it would theoretically be fine for the
> > > > > > compiler to generate double-precision float instructions for
> > > > > > computations as long as values are passed/returned according to LP64F
> > > > > > rules.
> > > > >
> > > > > If you're building code for -sf or -sp ABI, but could run on a machine
> > > > > with larger floating point register file, it's possible that the user
> > > > > could have libc built not to use fp registers at all or only 32-bit
> > > > > registers (respectively), but the calling application could be built
> > > > > for and running on a machine with 64-bit registers. In this case we
> > > > > need to understand what the ABI says. Are the 64-bit register, if
> > > > > present, call-saved in lower ABI levels where they don't participate
> > > > > in the calling convention? If so, no #ifdef is sufficient and there
> > > > > must be a runtime hwcap check here to determine which form of
> > > > > save/restore to do, like on arm and powerpc.
> > > >
> > > > I don't think this is a scenario that the ABI considers to be
> > > > supported. If you try to link code of different ABIs, you will get
> > > > linker errors such as:
> > > >
> > > >     /tmp/ccz2Y86f.o: can't link soft-float modules with double-float modules
> > > >
> > > > or
> > > >
> > > >     /tmp/cc7rTh7R.o: can't link single-float modules with double-float modules
> > >
> > > Those are different ABIs. You can't link modules with mismatched ABI,
> > > but you should be able to link modules that are both using -sf ABI (or
> > > both using -sp ABI), where one is not using the fpu and the other is
> > > using the full double fpu but only passing args in GPRs to conform
> > > with the ABI. If that's not allowed, I would consider it a tooling
> > > bug; there's no compatibility-constraint reason it can't be allowed.
> >
> > Oh, of course. I misread your question, sorry.
> >
> > Here's, I think, the relevant section of the calling convention:
> > https://github.com/riscv-non-isa/riscv-elf-psabi-doc/blob/master/riscv-cc.adoc#floating-point-register-convention
> >
> > This part is a bit awkward (to me, at least):
> >
> >     Floating-point values in callee-saved registers are only preserved
> > across calls if they are no larger than the width of a floating-point
> > register in the targeted ABI. Therefore, these registers can always be
> > considered temporaries if targeting the base integer calling
> > convention.
> >
> > I'm not really sure why they're talking about "values" there; I would
> > think the register width (in the machine vs the ABI) is the only thing
> > we're concerned about in this context. I'm assuming that what they
> > mean is:
> >
> >     Floating-point registers in the callee-saved set are only
> > preserved across calls if they are no larger than the width of a
> > floating-point register in the targeted ABI.
>
> OK, perfect. That means we only need to decide what to save based on
> the ABI, not dynamic hwcap or FPU capabilities.

Does that mean the patch is fine as-is, or are there any adjustments I
should make?