[libc][math][c23] Add entrypoints and tests for fsqrt{,l,f128} #99669

ghost · 2024-07-19T17:25:01Z

Hello!

My initial pr consists of entry points and the functions fsqrt, fsqrtl, fsqrtf128. These are the instances of the sqrt template in libc/src/__support/FPUtil/generic/.

Next, I will add the tests.

llvmbot · 2024-07-19T17:25:35Z

@llvm/pr-subscribers-libc

Author: Job Henandez Lara (Jobhdez)

Changes

Hello!

My initial pr consists of entry points and the functions fsqrt, fsqrtl, fsqrtf128. These are the instances of the sqrt template in libc/src/__support/FPUtil/generic/.

Next, I will add the tests.

Full diff: https://github.com/llvm/llvm-project/pull/99669.diff

9 Files Affected:

(modified) libc/config/linux/x86_64/entrypoints.txt (+3)
(modified) libc/src/math/CMakeLists.txt (+4)
(added) libc/src/math/fsqrt.h (+21)
(added) libc/src/math/fsqrtf128.h (+21)
(added) libc/src/math/fsqrtl.h (+21)
(modified) libc/src/math/generic/CMakeLists.txt (+39)
(added) libc/src/math/generic/fsqrt.cpp (+18)
(added) libc/src/math/generic/fsqrtf128.cpp (+20)
(added) libc/src/math/generic/fsqrtl.cpp (+20)

diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index cbdee084aa199..9705f5cde3798 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -448,6 +448,8 @@ set(TARGET_LIBM_ENTRYPOINTS
     libc.src.math.fromfpx
     libc.src.math.fromfpxf
     libc.src.math.fromfpxl
+    libc.src.math.fsqrt
+    libc.src.math.fsqrtl
     libc.src.math.hypot
     libc.src.math.hypotf
     libc.src.math.ilogb
@@ -659,6 +661,7 @@ if(LIBC_TYPES_HAS_FLOAT128)
     libc.src.math.frexpf128
     libc.src.math.fromfpf128
     libc.src.math.fromfpxf128
+    libc.src.math.fsqrtf128
     libc.src.math.ilogbf128
     libc.src.math.ldexpf128
     libc.src.math.llogbf128
diff --git a/libc/src/math/CMakeLists.txt b/libc/src/math/CMakeLists.txt
index c4e33130e9090..cec3cd3337260 100644
--- a/libc/src/math/CMakeLists.txt
+++ b/libc/src/math/CMakeLists.txt
@@ -131,6 +131,10 @@ add_math_entrypoint_object(f16sqrtf)
 add_math_entrypoint_object(f16sqrtl)
 add_math_entrypoint_object(f16sqrtf128)
 
+add_math_entrypoint_object(fsqrt)
+add_math_entrypoint_object(fsqrtl)
+add_math_entrypoint_object(fsqrtf128)
+
 add_math_entrypoint_object(f16sub)
 add_math_entrypoint_object(f16subf)
 add_math_entrypoint_object(f16subl)
diff --git a/libc/src/math/fsqrt.h b/libc/src/math/fsqrt.h
new file mode 100644
index 0000000000000..1063d2e38db75
--- /dev/null
+++ b/libc/src/math/fsqrt.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for fsqrt -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_FSQRT_H
+#define LLVM_LIBC_SRC_MATH_FSQRT_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+float fsqrt(double x);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_MATH_F16SQRT_H
diff --git a/libc/src/math/fsqrtf128.h b/libc/src/math/fsqrtf128.h
new file mode 100644
index 0000000000000..5286cbc33116d
--- /dev/null
+++ b/libc/src/math/fsqrtf128.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for fsqrtf128 ---------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_FSQRTF128_H
+#define LLVM_LIBC_SRC_MATH_FSQRTF128_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+float fsqrtf128(float128 x);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_MATH_FSQRTF128_H
diff --git a/libc/src/math/fsqrtl.h b/libc/src/math/fsqrtl.h
new file mode 100644
index 0000000000000..a857bab1d1ed6
--- /dev/null
+++ b/libc/src/math/fsqrtl.h
@@ -0,0 +1,21 @@
+//===-- Implementation header for fsqrtl ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_FSQRTL_H
+#define LLVM_LIBC_SRC_MATH_FSQRTL_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+float fsqrtl(long double x);
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_MATH_FSQRTL_H
diff --git a/libc/src/math/generic/CMakeLists.txt b/libc/src/math/generic/CMakeLists.txt
index d775026fabb3e..ccb34119b1df0 100644
--- a/libc/src/math/generic/CMakeLists.txt
+++ b/libc/src/math/generic/CMakeLists.txt
@@ -4189,6 +4189,45 @@ add_entrypoint_object(
     -O3
 )
 
+add_entrypoint_object(
+  fsqrt
+  SRCS
+    fsqrt.cpp
+  HDRS
+    ../fsqrt.h
+  DEPENDS
+    libc.src.__support.macros.properties.types
+    libc.src.__support.FPUtil.generic.sqrt
+  COMPILE_OPTIONS
+    -O3
+)
+
+add_entrypoint_object(
+  fsqrtl
+  SRCS
+    fsqrtl.cpp
+  HDRS
+    ../fsqrtl.h
+  DEPENDS
+    libc.src.__support.macros.properties.types
+    libc.src.__support.FPUtil.generic.sqrt
+  COMPILE_OPTIONS
+    -O3
+)
+
+add_entrypoint_object(
+  fsqrtf128
+  SRCS
+    fsqrtf128.cpp
+  HDRS
+    ../fsqrtf128.h
+  DEPENDS
+    libc.src.__support.macros.properties.types
+    libc.src.__support.FPUtil.generic.sqrt
+  COMPILE_OPTIONS
+    -O3
+)
+
 add_entrypoint_object(
   cbrtf
   SRCS
diff --git a/libc/src/math/generic/fsqrt.cpp b/libc/src/math/generic/fsqrt.cpp
new file mode 100644
index 0000000000000..d54471fd067bf
--- /dev/null
+++ b/libc/src/math/generic/fsqrt.cpp
@@ -0,0 +1,18 @@
+//===-- Implementation of fsqrt function ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/fsqrt.h"
+#include "src/__support/FPUtil/generic/sqrt.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(float, fsqrt, (double x)) { return fputil::sqrt<float>(x); }
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/math/generic/fsqrtf128.cpp b/libc/src/math/generic/fsqrtf128.cpp
new file mode 100644
index 0000000000000..f2c049529d6cd
--- /dev/null
+++ b/libc/src/math/generic/fsqrtf128.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation of fsqrt128 function -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/fsqrtf128.h"
+#include "src/__support/FPUtil/generic/sqrt.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(float, fsqrtf128, (float128 x)) {
+  return fputil::sqrt<float>(x);
+}
+
+} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/math/generic/fsqrtl.cpp b/libc/src/math/generic/fsqrtl.cpp
new file mode 100644
index 0000000000000..b896a84aeded8
--- /dev/null
+++ b/libc/src/math/generic/fsqrtl.cpp
@@ -0,0 +1,20 @@
+//===-- Implementation of fsqrtl function ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/fsqrtl.h"
+#include "src/__support/FPUtil/generic/sqrt.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/config.h"
+
+namespace LIBC_NAMESPACE_DECL {
+
+LLVM_LIBC_FUNCTION(float, fsqrtl, (long double x)) {
+  return fputil::sqrt<float>(x);
+}
+
+} // namespace LIBC_NAMESPACE_DECL

lntue · 2024-07-19T19:08:42Z

libc/src/math/fsqrt.h

+#define LLVM_LIBC_SRC_MATH_FSQRT_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"


src/__support/macros/properties/types.h header is not needed here.

lntue · 2024-07-19T19:09:09Z

libc/src/math/fsqrt.h

+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_SRC_MATH_F16SQRT_H


LLVM_LIBC_SRC_MATH_FSQRT_H

lntue · 2024-07-19T19:09:32Z

libc/src/math/fsqrtl.h

+#define LLVM_LIBC_SRC_MATH_FSQRTL_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/types.h"


src/__support/macros/properties/types.h header is not needed here.

lntue · 2024-07-19T19:10:11Z

libc/src/math/generic/CMakeLists.txt

+  HDRS
+    ../fsqrt.h
+  DEPENDS
+    libc.src.__support.macros.properties.types


libc.src.__support.macros.properties.types dependency is not needed

lntue · 2024-07-19T19:10:15Z

libc/src/math/generic/CMakeLists.txt

+  HDRS
+    ../fsqrtl.h
+  DEPENDS
+    libc.src.__support.macros.properties.types


libc.src.__support.macros.properties.types dependency is not needed

lntue · 2024-07-20T18:27:26Z

libc/test/src/math/smoke/f16sqrtf128_test.cpp

@@ -1,4 +1,4 @@
-//===-- Unittests for f16sqrtf128 -----------------------------------------===//


this file shouldn't be changed.

lntue · 2024-07-20T18:28:28Z

libc/spec/stdc.td

@@ -739,6 +739,10 @@ def StdC : StandardSpec<"stdc"> {

          GuardedFunctionSpec<"f16mulf128", RetValSpec<Float16Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT16_AND_FLOAT128">,

+          FunctionSpec<"fsqrt", RetValSpec<FloatType>, [ArgSpec<DoubleType>]>,
+          FunctionSpec<"fsqrtl", RetValSpec<FloatType>, [ArgSpec<LongDoubleType>]>,
+	  GuardedFunctionSpec<"f16sqrtf128", RetValSpec<FloatType>, [ArgSpec<Float128Type>]>,


fsqrtf128, and the header guard is missing.

overmighty

Could you please update the implementation status table in the docs? Since fsqrtf128 is an LLVM libc extension, it should have a |check|\* and not just a |check| like the others.

libc/src/math/fsqrtf128.h

overmighty · 2024-07-20T22:11:17Z

libc/src/math/generic/CMakeLists.txt

+  DEPENDS
+    libc.src.__support.FPUtil.generic.sqrt


Suggested change

DEPENDS

libc.src.__support.FPUtil.generic.sqrt

DEPENDS

libc.src.__support.macros.properties.types

libc.src.__support.FPUtil.generic.sqrt

tue told me remove libc.src.__support.macros.properties.types

overmighty · 2024-07-20T22:12:58Z

libc/test/src/math/smoke/fsqrt_test.cpp

@@ -0,0 +1,13 @@
+//===-- Unittests for fsqrt -------------------------------------------===//


Nit: formatting.

Suggested change

//===-- Unittests for fsqrt -------------------------------------------===//

//===-- Unittests for fsqrt -----------------------------------------------===//

overmighty · 2024-07-20T22:13:29Z

libc/test/src/math/smoke/fsqrtl_test.cpp

@@ -0,0 +1,13 @@
+//===-- Unittests for fsqrtl ------------------------------------------===//


Nit: formatting.

Suggested change

//===-- Unittests for fsqrtl ------------------------------------------===//

//===-- Unittests for fsqrtl ----------------------------------------------===//

overmighty · 2024-07-20T22:17:14Z

libc/spec/stdc.td

@@ -739,6 +739,10 @@ def StdC : StandardSpec<"stdc"> {

          GuardedFunctionSpec<"f16mulf128", RetValSpec<Float16Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT16_AND_FLOAT128">,

+          FunctionSpec<"fsqrt", RetValSpec<FloatType>, [ArgSpec<DoubleType>]>,
+          FunctionSpec<"fsqrtl", RetValSpec<FloatType>, [ArgSpec<LongDoubleType>]>,
+	  GuardedFunctionSpec<"fsqrtf128", RetValSpec<FloatType>, [ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,


fsqrtf128 should be an LLVM libc extension, so it should be moved to libc/spec/llvm_libc_ext.td. The C standard only defines these:

float fsqrt(double x); float fsqrtl(long double x); _FloatM fMsqrtfN(_FloatN x); // M < N

overmighty · 2024-07-20T22:35:33Z

I've changed the PR title to better reflect the fact that it adds multiple functions/entrypoints, if you don't mind.

ghost · 2024-07-21T00:24:43Z

I've changed the PR title to better reflect the fact that it adds multiple functions/entrypoints, if you don't mind.

its ok

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251564

llvmbot added the libc label Jul 19, 2024

lntue reviewed Jul 19, 2024

View reviewed changes

Job Hernandez added 3 commits July 20, 2024 11:00

add entrypoints and functions

10214f2

format code

8be52c2

add tests

e2f1047

lntue reviewed Jul 20, 2024

View reviewed changes

Job Hernandez added 3 commits July 20, 2024 14:16

add template for explain and compare

4c226d6

format code

d72ce9d

address review

9180d3e

lntue requested a review from overmighty July 20, 2024 22:04

overmighty reviewed Jul 20, 2024

View reviewed changes

overmighty changed the title ~~[libc][math][c23] add entry points and tests for fsqrt~~ [libc][math][c23] Add entrypoints and tests for fsqrt{,l,f128} Jul 20, 2024

address review

1160f18

ghost requested review from overmighty and lntue July 21, 2024 00:42

lntue approved these changes Jul 21, 2024

View reviewed changes

lntue merged commit af0f58c into llvm:main Jul 21, 2024
7 checks passed

ghost mentioned this pull request Jul 21, 2024

[libc][math][c23] add entrypoints and tests for fsqrt and dsqrt #99600

Closed

2 tasks

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024

[libc][math][c23] Add entrypoints and tests for fsqrt{,l,f128} (#99669)

582821d

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251564


		} // namespace LIBC_NAMESPACE_DECL

		#endif // LLVM_LIBC_SRC_MATH_F16SQRT_H

		@@ -1,4 +1,4 @@
		//===-- Unittests for f16sqrtf128 -----------------------------------------===//

		@@ -0,0 +1,13 @@
		//===-- Unittests for fsqrt -------------------------------------------===//

		@@ -0,0 +1,13 @@
		//===-- Unittests for fsqrtl ------------------------------------------===//

[libc][math][c23] Add entrypoints and tests for fsqrt{,l,f128} #99669

[libc][math][c23] Add entrypoints and tests for fsqrt{,l,f128} #99669

Uh oh!

Conversation

ghost commented Jul 19, 2024

Uh oh!

llvmbot commented Jul 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overmighty left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

overmighty commented Jul 20, 2024

Uh oh!

ghost commented Jul 21, 2024

Uh oh!

Uh oh!

Uh oh!