clang/OpenCL: Add inline implementations of sqrt in builtin header

[arsenm]

Summary

We want the !fpmath metadata to be attached to the sqrt intrinsic to
make it to the backend lowering. Emit an available_externally
definition which uses the builtin, which emits the !fpmath.

Fixes #64264

[arsenm]

Move to base header and fix test. update_cc_test_checks --include-generated-funcs does a terrible job here so write manually checks

[Anastasia]

clang/lib/Headers/opencl-c-base.h

832 ↗

(On Diff #546164)

Is this a generic implementation enough? Would some targets not need to do something different for this built-in?

Ideally this header is to be kept light so I am a bit worried about adding definitions of the functions here. Otherwise we will end up in the same situation as we one day were with opencl-c.h. So could these be left there instead? It might be good to check with @svenvh if TableGen header has already a way to do this function forwarding or can be extended to do such a thing. Then it would be implementable in the both header mechanisms. I don't know if Sven has some other ideas or opinions...

[svenvh]

If we go with this approach, please also remove sqrt from clang/lib/Sema/OpenCLBuiltins.td (and ideally add a comment pointing out that sqrt is handled in opencl-c-base.h)

clang/lib/Headers/opencl-c-base.h

832 ↗

(On Diff #546164)

We did already discuss this a bit on the GitHub issue: https://github.com/llvm/llvm-project/issues/64264

856–858 ↗

(On Diff #546164)

[arsenm]

clang/lib/Headers/opencl-c-base.h

832 ↗

(On Diff #546164)

As I mentioned on the ticket, it's only this one case so I'm not worried about adding a lot more to the base header. I think we can start by assuming llvm.sqrt always works correctly, I don't want to add more complexity to handle this case without a specific reason

[Anastasia]

clang/lib/Headers/opencl-c-base.h

832 ↗

(On Diff #546164)

As I mentioned on the ticket, it's only this one case so I'm not worried about adding a lot more to the base header.

This is how things normally start. Someone else might want to continue this approach because it is already there.

I think we can start by assuming llvm.sqrt always works correctly, I don't want to add more complexity to handle this case without a specific reason

Do you mean it would apply to all implementations? What I am missing here is why it is required to be in the headers? Is this because it needs to be inlined or is it because the compiler must see __builtin_elementwise_sqrt with the surrounding code where it is called from?

[arsenm]

clang/lib/Headers/opencl-c-base.h

832 ↗

(On Diff #546164)

Yes, I would expect any llvm consumer to correctly lower llvm.sqrt, and such an implementation would be correctly rounded and pass conformance with -cl-fp32-correctly-rounded-divide-sqrt

The goal is to get !fpmath attached to an llvm.sqrt call. The way this currently happens is based on the language, it gets emitted from the various __builtin_sqrt* calls

[arsenm]

ping

The alternative is to directly put the !fpmath on the sqrt call sites but I have no idea how to do that

[arsenm]

ping

[arsenm]

ping

[Anastasia]

If we think there are no better alternatives and implementation is generic enough for every vendor, LGTM!

[arsenm]

In D156743#4644285, @Anastasia wrote:

If we think there are no better alternatives and implementation is generic enough for every vendor, LGTM!

You could argue annotating the raw callsite is better but I don't know how to implement that

[arsenm]

15e0fe0b6122e32657b98daf74a1fce028d2e5bf

clang/lib/Headers/opencl-c.h

 half3 __ovld __cnfn sinpi(half3);
 half4 __ovld __cnfn sinpi(half4);
 half8 __ovld __cnfn sinpi(half8);
 half16 __ovld __cnfn sinpi(half16);
 #endif //cl_khr_fp16
 
 /**
  * Compute square root.
- */
-float __ovld __cnfn sqrt(float);
-float2 __ovld __cnfn sqrt(float2);
-float3 __ovld __cnfn sqrt(float3);
-float4 __ovld __cnfn sqrt(float4);
-float8 __ovld __cnfn sqrt(float8);
-float16 __ovld __cnfn sqrt(float16);
+ *
+ * Provide inline implementations using the builtin so that we get appropriate
+ * !fpmath based on -cl-fp32-correctly-rounded-divide-sqrt. The implementation
+ * should still provide an external definition.
+ */
+inline float __ovld __cnfn sqrt(float __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
+inline float2 __ovld __cnfn sqrt(float2 __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
+inline float3 __ovld __cnfn sqrt(float3 __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
+inline float4 __ovld __cnfn sqrt(float4 __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
+inline float8 __ovld __cnfn sqrt(float8 __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
+inline float16 __ovld __cnfn sqrt(float16 __x) {
+  return __builtin_elementwise_sqrt(__x);
+}
+
 #ifdef cl_khr_fp64
 double __ovld __cnfn sqrt(double);
 double2 __ovld __cnfn sqrt(double2);
 double3 __ovld __cnfn sqrt(double3);
 double4 __ovld __cnfn sqrt(double4);
 double8 __ovld __cnfn sqrt(double8);
 double16 __ovld __cnfn sqrt(double16);
 #endif //cl_khr_fp64

clang/test/CodeGenOpenCL/sqrt-fpmath.cl

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 2
+// REQUIRES: amdgpu-registered-target
+
+// Test with -fdeclare-opencl-builtins
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -fdeclare-opencl-builtins -finclude-default-header -target-cpu hawaii -S -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -fdeclare-opencl-builtins -finclude-default-header -cl-fp32-correctly-rounded-divide-sqrt -target-cpu hawaii -S -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,CORRECTLYROUNDED %s
+
+// Test without -fdeclare-opencl-builtins
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -finclude-default-header -target-cpu hawaii -S -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,DEFAULT %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -finclude-default-header -cl-fp32-correctly-rounded-divide-sqrt -target-cpu hawaii -S -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK,CORRECTLYROUNDED %s
+
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
+
+// CHECK-LABEL: define dso_local float @call_sqrt_f32
+// CHECK-SAME: (float noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call float @_Z4sqrtf(float noundef [[X]]) #[[ATTR2:[0-9]+]]
+// CHECK-NEXT:    ret float [[CALL]]
+//
+float call_sqrt_f32(float x) {
+  return sqrt(x);
+}
+
+// CHECK-LABEL: define dso_local <2 x float> @call_sqrt_v2f32
+// CHECK-SAME: (<2 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <2 x float> @_Z4sqrtDv2_f(<2 x float> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <2 x float> [[CALL]]
+//
+float2 call_sqrt_v2f32(float2 x) {
+  return sqrt(x);
+}
+
+// CHECK-LABEL: define dso_local <3 x float> @call_sqrt_v3f32
+// CHECK-SAME: (<3 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <3 x float> @_Z4sqrtDv3_f(<3 x float> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <3 x float> [[CALL]]
+//
+float3 call_sqrt_v3f32(float3 x) {
+  return sqrt(x);
+}
+
+// CHECK-LABEL: define dso_local <4 x float> @call_sqrt_v4f32
+// CHECK-SAME: (<4 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <4 x float> @_Z4sqrtDv4_f(<4 x float> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <4 x float> [[CALL]]
+//
+float4 call_sqrt_v4f32(float4 x) {
+  return sqrt(x);
+}
+
+// CHECK-LABEL: define dso_local <8 x float> @call_sqrt_v8f32
+// CHECK-SAME: (<8 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <8 x float> @_Z4sqrtDv8_f(<8 x float> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <8 x float> [[CALL]]
+//
+float8 call_sqrt_v8f32(float8 x) {
+  return sqrt(x);
+}
+
+// CHECK-LABEL: define dso_local <16 x float> @call_sqrt_v16f32
+// CHECK-SAME: (<16 x float> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <16 x float> @_Z4sqrtDv16_f(<16 x float> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <16 x float> [[CALL]]
+//
+float16 call_sqrt_v16f32(float16 x) {
+  return sqrt(x);
+}
+
+// Not for f64
+// CHECK-LABEL: define dso_local double @call_sqrt_f64
+// CHECK-SAME: (double noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call double @_Z4sqrtd(double noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret double [[CALL]]
+//
+double call_sqrt_f64(double x) {
+  return sqrt(x);
+}
+
+// Not for f64
+// CHECK-LABEL: define dso_local <2 x double> @call_sqrt_v2f64
+// CHECK-SAME: (<2 x double> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <2 x double> @_Z4sqrtDv2_d(<2 x double> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <2 x double> [[CALL]]
+//
+double2 call_sqrt_v2f64(double2 x) {
+  return sqrt(x);
+}
+
+// Not for f64
+// CHECK-LABEL: define dso_local half @call_sqrt_f16
+// CHECK-SAME: (half noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call half @_Z4sqrtDh(half noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret half [[CALL]]
+//
+half call_sqrt_f16(half x) {
+  return sqrt(x);
+}
+
+// Not for f64
+// CHECK-LABEL: define dso_local <2 x half> @call_sqrt_v2f16
+// CHECK-SAME: (<2 x half> noundef [[X:%.*]]) local_unnamed_addr #[[ATTR0]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[CALL:%.*]] = tail call <2 x half> @_Z4sqrtDv2_Dh(<2 x half> noundef [[X]]) #[[ATTR2]]
+// CHECK-NEXT:    ret <2 x half> [[CALL]]
+//
+half2 call_sqrt_v2f16(half2 x) {
+  return sqrt(x);
+}
+//// NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+// CORRECTLYROUNDED: {{.*}}
+// DEFAULT: {{.*}}