[musl] [PATCH v3] math: add riscv64 round/roundf

[musl] [PATCH v3] math: add riscv64 round/roundf

[Meng Zhuo]

---
First of all, sorry for any inconvenience.
This is my first time try to contribute with mail list.
---
v1 -> v3:
* using isfinite to check NaN,+-inf and in range without raising INVALID
* inlined copysign(f)
---
 src/math/riscv64/round.c  | 20 ++++++++++++++++++++
 src/math/riscv64/roundf.c | 20 ++++++++++++++++++++
 2 files changed, 40 insertions(+)
 create mode 100644 src/math/riscv64/round.c
 create mode 100644 src/math/riscv64/roundf.c

diff --git a/src/math/riscv64/round.c b/src/math/riscv64/round.c
new file mode 100644
index 00000000..6d91c093
--- /dev/null
+++ b/src/math/riscv64/round.c
@@ -0,0 +1,20 @@
+#include <math.h>
+
+#if __riscv_flen >= 64
+
+double round(double x)
+{
+	if (!isfinite(x)) return x;
+	double tmp;
+	long long n;
+	__asm__ ("fcvt.l.d %0, %1, rmm" : "=r"(n) : "f"(x));
+	__asm__ ("fcvt.d.l %0, %1" : "=f"(tmp) : "r"(n));
+	__asm__ ("fsgnj.d %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));
+	return x;
+}
+
+#else
+
+#include "../round.c"
+
+#endif
diff --git a/src/math/riscv64/roundf.c b/src/math/riscv64/roundf.c
new file mode 100644
index 00000000..e67e791f
--- /dev/null
+++ b/src/math/riscv64/roundf.c
@@ -0,0 +1,20 @@
+#include <math.h>
+
+#if __riscv_flen >= 32
+
+float roundf(float x)
+{
+	if (!isfinite(x)) return x;
+	float tmp;
+	long n;
+	__asm__ ("fcvt.w.s %0, %1, rmm" : "=r"(n) : "f"(x));
+	__asm__ ("fcvt.s.w %0, %1" : "=f"(tmp) : "r"(n));
+	__asm__ ("fsgnj.s %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));
+	return x;
+}
+
+#else
+
+#include "../roundf.c"
+
+#endif
-- 
2.39.2

Re: [musl] [PATCH v3] math: add riscv64 round/roundf

[Szabolcs Nagy]

* Meng Zhuo <mzh@mzh.io> [2024-06-13 00:06:12 +0800]:

> ---
> First of all, sorry for any inconvenience.
> This is my first time try to contribute with mail list.
> ---
> v1 -> v3:
> * using isfinite to check NaN,+-inf and in range without raising INVALID
> * inlined copysign(f)
> ---
>  src/math/riscv64/round.c  | 20 ++++++++++++++++++++
>  src/math/riscv64/roundf.c | 20 ++++++++++++++++++++
>  2 files changed, 40 insertions(+)
>  create mode 100644 src/math/riscv64/round.c
>  create mode 100644 src/math/riscv64/roundf.c
> 
> diff --git a/src/math/riscv64/round.c b/src/math/riscv64/round.c
> new file mode 100644
> index 00000000..6d91c093
> --- /dev/null
> +++ b/src/math/riscv64/round.c
> @@ -0,0 +1,20 @@
> +#include <math.h>
> +
> +#if __riscv_flen >= 64
> +
> +double round(double x)
> +{
> +	if (!isfinite(x)) return x;

!isfinite(x) && fabs(x) >= 0x1p52

otherwise n can overflow below.

(note that the check can be a single cmp+branch on the bit representation of x)

> +	double tmp;
> +	long long n;
> +	__asm__ ("fcvt.l.d %0, %1, rmm" : "=r"(n) : "f"(x));
> +	__asm__ ("fcvt.d.l %0, %1" : "=f"(tmp) : "r"(n));
> +	__asm__ ("fsgnj.d %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));

i'd add a comment that the sign bit is only copied to handle round(-0.0)

> +	return x;
> +}
> +
> +#else
> +
> +#include "../round.c"
> +
> +#endif
> diff --git a/src/math/riscv64/roundf.c b/src/math/riscv64/roundf.c
> new file mode 100644
> index 00000000..e67e791f
> --- /dev/null
> +++ b/src/math/riscv64/roundf.c
> @@ -0,0 +1,20 @@
> +#include <math.h>
> +
> +#if __riscv_flen >= 32
> +
> +float roundf(float x)
> +{
> +	if (!isfinite(x)) return x;

&& fabs(x) >= 0x1p23

> +	float tmp;
> +	long n;
> +	__asm__ ("fcvt.w.s %0, %1, rmm" : "=r"(n) : "f"(x));
> +	__asm__ ("fcvt.s.w %0, %1" : "=f"(tmp) : "r"(n));
> +	__asm__ ("fsgnj.s %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));
> +	return x;
> +}
> +
> +#else
> +
> +#include "../roundf.c"
> +
> +#endif
> -- 
> 2.39.2

Re: [musl] [PATCH v3] math: add riscv64 round/roundf

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2024-06-12 18:50:04 +0200]:
> > +double round(double x)
> > +{
> > +	if (!isfinite(x)) return x;
> 
> !isfinite(x) && fabs(x) >= 0x1p52

i meant ||, not &&

> 
> otherwise n can overflow below.
> 
> (note that the check can be a single cmp+branch on the bit representation of x)

[musl] [PATCH v3] math: add riscv64 round/roundf

[Meng Zhuo]

---
v2 -> v3:
 * use x + x to check +-inf and NaN
---
v1 -> v2:
 * drop ±inf check and use fabs as Rich suggested
---
 src/math/riscv64/round.c  | 21 +++++++++++++++++++++
 src/math/riscv64/roundf.c | 21 +++++++++++++++++++++
 2 files changed, 42 insertions(+)
 create mode 100644 src/math/riscv64/round.c
 create mode 100644 src/math/riscv64/roundf.c

diff --git a/src/math/riscv64/round.c b/src/math/riscv64/round.c
new file mode 100644
index 00000000..28d05aed
--- /dev/null
+++ b/src/math/riscv64/round.c
@@ -0,0 +1,21 @@
+#include <math.h>
+
+#if __riscv_flen >= 64
+
+double round(double x)
+{
+	if (isnan(x + x)) return x; // if x is +-inf or nan
+	if (fabs(x) >= 0x1p54) return x;
+	double tmp;
+	long long n;
+	__asm__ ("fcvt.l.d %0, %1, rmm" : "=r"(n) : "f"(x));
+	__asm__ ("fcvt.d.l %0, %1" : "=f"(tmp) : "r"(n));
+	__asm__ ("fsgnj.d %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));
+	return x;
+}
+
+#else
+
+#include "../round.c"
+
+#endif
diff --git a/src/math/riscv64/roundf.c b/src/math/riscv64/roundf.c
new file mode 100644
index 00000000..a42b00d9
--- /dev/null
+++ b/src/math/riscv64/roundf.c
@@ -0,0 +1,21 @@
+#include <math.h>
+
+#if __riscv_flen >= 32
+
+float roundf(float x)
+{
+	if (isnan(x + x)) return x;
+	if (fabsf(x) >= 0x1p25) return x;
+	float tmp;
+	long n;
+	__asm__ ("fcvt.w.s %0, %1, rmm" : "=r"(n) : "f"(x));
+	__asm__ ("fcvt.s.w %0, %1" : "=f"(tmp) : "r"(n));
+	__asm__ ("fsgnj.s %0, %1, %2" : "=f"(x) : "f"(tmp), "f"(x));
+	return x;
+}
+
+#else
+
+#include "../roundf.c"
+
+#endif
-- 
2.39.2

Re: [musl] [PATCH v3] math: add riscv64 round/roundf

[Rich Felker]

On Wed, Jun 12, 2024 at 11:31:05PM +0800, Meng Zhuo wrote:
> ---
> v2 -> v3:
>  * use x + x to check +-inf and NaN
> ---
> v1 -> v2:
>  * drop ±inf check and use fabs as Rich suggested
> ---
>  src/math/riscv64/round.c  | 21 +++++++++++++++++++++
>  src/math/riscv64/roundf.c | 21 +++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>  create mode 100644 src/math/riscv64/round.c
>  create mode 100644 src/math/riscv64/roundf.c
> 
> diff --git a/src/math/riscv64/round.c b/src/math/riscv64/round.c
> new file mode 100644
> index 00000000..28d05aed
> --- /dev/null
> +++ b/src/math/riscv64/round.c
> @@ -0,0 +1,21 @@
> +#include <math.h>
> +
> +#if __riscv_flen >= 64
> +
> +double round(double x)
> +{
> +	if (isnan(x + x)) return x; // if x is +-inf or nan
> +	if (fabs(x) >= 0x1p54) return x;

Why? I already gave you a single expression that covers all these
cases if you don't care about raising INVALID (I don't recall if
that's allowed here):

	if (!(fabs(x) < 0x1p54)) return x;

Your new x+x *does* raise INVALID, so it has no advantages.

If the function needs to avoid raising INVALID, you need to check
isnan(x) (or !isfinite(x) is probably better) separately like you were
doing. We need to check this.

Rich