clang: Fix handling of __builtin_elementwise_copysign

arsenm · arsenm · commit 2ad4c3c88d88 · 2023-01-10T14:45:23.000-05:00
I realized the handling of copysign made no sense at all.
Only the type of the first operand should really matter, and
it should not perform a conversion between them.

Also fixes misleading errors and producing broken IR for
integers.

We could accept different FP types for the sign argument,
if the intrinsic permitted different types like the DAG node.
As it is we would need to insert a cast, which could have
other effects (like trapping on snan) which should not happen
for a copysign.
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -2025,6 +2025,33 @@ bool Sema::CheckTSBuiltinFunctionCall(const TargetInfo &TI, unsigned BuiltinID,
   }
 }
 
+// Check if \p Ty is a valid type for the elementwise math builtins. If it is
+// not a valid type, emit an error message and return true. Otherwise return
+// false.
+static bool checkMathBuiltinElementType(Sema &S, SourceLocation Loc,
+                                        QualType Ty) {
+  if (!Ty->getAs<VectorType>() && !ConstantMatrixType::isValidElementType(Ty)) {
+    return S.Diag(Loc, diag::err_builtin_invalid_arg_type)
+           << 1 << /* vector, integer or float ty*/ 0 << Ty;
+  }
+
+  return false;
+}
+
+static bool checkFPMathBuiltinElementType(Sema &S, SourceLocation Loc,
+                                          QualType ArgTy, int ArgIndex) {
+  QualType EltTy = ArgTy;
+  if (auto *VecTy = EltTy->getAs<VectorType>())
+    EltTy = VecTy->getElementType();
+
+  if (!EltTy->isRealFloatingType()) {
+    return S.Diag(Loc, diag::err_builtin_invalid_arg_type)
+           << ArgIndex << /* vector or float ty*/ 5 << ArgTy;
+  }
+
+  return false;
+}
+
 ExprResult
 Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
                                CallExpr *TheCall) {
@@ -2621,10 +2648,38 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID,
 
   case Builtin::BI__builtin_elementwise_min:
   case Builtin::BI__builtin_elementwise_max:
-  case Builtin::BI__builtin_elementwise_copysign:
     if (SemaBuiltinElementwiseMath(TheCall))
       return ExprError();
     break;
+  case Builtin::BI__builtin_elementwise_copysign: {
+    if (checkArgCount(*this, TheCall, 2))
+      return ExprError();
+
+    ExprResult Magnitude = UsualUnaryConversions(TheCall->getArg(0));
+    ExprResult Sign = UsualUnaryConversions(TheCall->getArg(1));
+    if (Magnitude.isInvalid() || Sign.isInvalid())
+      return ExprError();
+
+    QualType MagnitudeTy = Magnitude.get()->getType();
+    QualType SignTy = Sign.get()->getType();
+    if (checkFPMathBuiltinElementType(*this, TheCall->getArg(0)->getBeginLoc(),
+                                      MagnitudeTy, 1) ||
+        checkFPMathBuiltinElementType(*this, TheCall->getArg(1)->getBeginLoc(),
+                                      SignTy, 2)) {
+      return ExprError();
+    }
+
+    if (MagnitudeTy.getCanonicalType() != SignTy.getCanonicalType()) {
+      return Diag(Sign.get()->getBeginLoc(),
+                  diag::err_typecheck_call_different_arg_types)
+             << MagnitudeTy << SignTy;
+    }
+
+    TheCall->setArg(0, Magnitude.get());
+    TheCall->setArg(1, Sign.get());
+    TheCall->setType(Magnitude.get()->getType());
+    break;
+  }
   case Builtin::BI__builtin_reduce_max:
   case Builtin::BI__builtin_reduce_min: {
     if (PrepareBuiltinReduceMathOneArgCall(TheCall))
@@ -17669,19 +17724,6 @@ void Sema::CheckAddressOfPackedMember(Expr *rhs) {
                      _2, _3, _4));
 }
 
-// Check if \p Ty is a valid type for the elementwise math builtins. If it is
-// not a valid type, emit an error message and return true. Otherwise return
-// false.
-static bool checkMathBuiltinElementType(Sema &S, SourceLocation Loc,
-                                        QualType Ty) {
-  if (!Ty->getAs<VectorType>() && !ConstantMatrixType::isValidElementType(Ty)) {
-    S.Diag(Loc, diag::err_builtin_invalid_arg_type)
-        << 1 << /* vector, integer or float ty*/ 0 << Ty;
-    return true;
-  }
-  return false;
-}
-
 bool Sema::PrepareBuiltinElementwiseMathOneArgCall(CallExpr *TheCall) {
   if (checkArgCount(*this, TheCall, 1))
     return true;
diff --git a/clang/test/CodeGen/builtins-elementwise-math.c b/clang/test/CodeGen/builtins-elementwise-math.c
@@ -432,7 +432,7 @@ void test_builtin_elementwise_canonicalize(float f1, float f2, double d1, double
 }
 
 void test_builtin_elementwise_copysign(float f1, float f2, double d1, double d2,
-                                       float4 vf1, float4 vf2) {
+                                       float4 vf1, float4 vf2, double2 v2f64) {
   // CHECK-LABEL: define void @test_builtin_elementwise_copysign(
   // CHECK:      [[F1:%.+]] = load float, ptr %f1.addr, align 4
   // CHECK-NEXT: [[F2:%.+]] = load float, ptr %f2.addr, align 4
@@ -463,4 +463,17 @@ void test_builtin_elementwise_copysign(float f1, float f2, double d1, double d2,
   // CHECK-NEXT: [[CVF1:%.+]] = load <4 x float>, ptr %cvf1, align 16
   // CHECK-NEXT: call <4 x float> @llvm.copysign.v4f32(<4 x float> [[VF2]], <4 x float> [[CVF1]])
   vf1 = __builtin_elementwise_copysign(vf2, cvf1);
+
+
+  // CHECK:      [[F1:%.+]] = load float, ptr %f1.addr
+  // CHECK-NEXT: call float @llvm.copysign.f32(float [[F1]], float 2.000000e+00)
+  f1 = __builtin_elementwise_copysign(f1, 2.0f);
+
+  // CHECK:      [[F1:%.+]] = load float, ptr %f1.addr
+  // CHECK-NEXT: call float @llvm.copysign.f32(float 2.000000e+00, float [[F1]])
+  f1 = __builtin_elementwise_copysign(2.0f, f1);
+
+  // CHECK:      [[V2F64:%.+]] = load <2 x double>, ptr %v2f64.addr, align 16
+  // CHECK-NEXT: call <2 x double> @llvm.copysign.v2f64(<2 x double> <double 1.000000e+00, double 1.000000e+00>, <2 x double> [[V2F64]])
+  v2f64 = __builtin_elementwise_copysign((double2)1.0, v2f64);
 }
diff --git a/clang/test/Sema/builtins-elementwise-math.c b/clang/test/Sema/builtins-elementwise-math.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -std=c99 %s -pedantic -verify -triple=x86_64-apple-darwin9
 
+typedef double double2 __attribute__((ext_vector_type(2)));
+typedef double double4 __attribute__((ext_vector_type(4)));
+typedef float float2 __attribute__((ext_vector_type(2)));
 typedef float float4 __attribute__((ext_vector_type(4)));
 typedef int int3 __attribute__((ext_vector_type(3)));
 typedef unsigned unsigned3 __attribute__((ext_vector_type(3)));
@@ -13,6 +16,11 @@ __attribute__((address_space(1))) int int_as_one;
 typedef int bar;
 bar b;
 
+__attribute__((address_space(1))) float float_as_one;
+typedef float waffle;
+waffle waf;
+
+
 void test_builtin_elementwise_abs(int i, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
   struct Foo s = __builtin_elementwise_abs(i);
   // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
@@ -406,12 +414,12 @@ void test_builtin_elementwise_canonicalize(int i, float f, double d, float4 v, i
   // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
 }
 
-void test_builtin_elementwise_copysign(int i, short s, double d, float4 v, int3 iv, unsigned3 uv, int *p) {
+void test_builtin_elementwise_copysign(int i, short s, double d, float f, float4 v, int3 iv, unsigned3 uv, int *p) {
   i = __builtin_elementwise_copysign(p, d);
-  // expected-error@-1 {{arguments are of different types ('int *' vs 'double')}}
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int *')}}
 
-  struct Foo foo = __builtin_elementwise_copysign(i, i);
-  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+  i = __builtin_elementwise_copysign(i, i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
 
   i = __builtin_elementwise_copysign(i);
   // expected-error@-1 {{too few arguments to function call, expected 2, have 1}}
@@ -423,40 +431,81 @@ void test_builtin_elementwise_copysign(int i, short s, double d, float4 v, int3
   // expected-error@-1 {{too many arguments to function call, expected 2, have 3}}
 
   i = __builtin_elementwise_copysign(v, iv);
-  // expected-error@-1 {{arguments are of different types ('float4' (vector of 4 'float' values) vs 'int3' (vector of 3 'int' values))}}
+  // expected-error@-1 {{2nd argument must be a floating point type (was 'int3' (vector of 3 'int' values))}}
 
   i = __builtin_elementwise_copysign(uv, iv);
-  // expected-error@-1 {{arguments are of different types ('unsigned3' (vector of 3 'unsigned int' values) vs 'int3' (vector of 3 'int' values))}}
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned3' (vector of 3 'unsigned int' values))}}
 
   s = __builtin_elementwise_copysign(i, s);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  f = __builtin_elementwise_copysign(f, i);
+  // expected-error@-1 {{2nd argument must be a floating point type (was 'int')}}
+
+  f = __builtin_elementwise_copysign(i, f);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
 
   enum e { one,
            two };
   i = __builtin_elementwise_copysign(one, two);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
 
   enum f { three };
   enum f x = __builtin_elementwise_copysign(one, three);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
 
   _BitInt(32) ext; // expected-warning {{'_BitInt' in C17 and earlier is a Clang extension}}
   ext = __builtin_elementwise_copysign(ext, ext);
+  // expected-error@-1 {{1st argument must be a floating point type (was '_BitInt(32)')}}
 
-  const int ci;
-  i = __builtin_elementwise_copysign(ci, i);
-  i = __builtin_elementwise_copysign(i, ci);
-  i = __builtin_elementwise_copysign(ci, ci);
+  const float cf32;
+  f = __builtin_elementwise_copysign(cf32, f);
+  f = __builtin_elementwise_copysign(f, cf32);
+  f = __builtin_elementwise_copysign(cf32, f);
 
-  i = __builtin_elementwise_copysign(i, int_as_one); // ok (attributes don't match)?
-  i = __builtin_elementwise_copysign(i, b);          // ok (sugar doesn't match)?
+  f = __builtin_elementwise_copysign(f, float_as_one); // ok (attributes don't match)?
+  f = __builtin_elementwise_copysign(f, waf);          // ok (sugar doesn't match)?
 
-  int A[10];
+  float A[10];
   A = __builtin_elementwise_copysign(A, A);
-  // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was 'int *')}}
+  // expected-error@-1 {{1st argument must be a floating point type (was 'float *')}}
 
-  int(ii);
-  int j;
-  j = __builtin_elementwise_copysign(i, j);
+  float(ii);
+  float j;
+  j = __builtin_elementwise_copysign(f, j);
 
   _Complex float c1, c2;
   c1 = __builtin_elementwise_copysign(c1, c2);
-  // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was '_Complex float')}}
+  // expected-error@-1 {{1st argument must be a floating point type (was '_Complex float')}}
+
+  double f64 = 0.0;
+  double tmp0 = __builtin_elementwise_copysign(f64, f);
+  // expected-error@-1 {{arguments are of different types ('double' vs 'float')}}
+
+  float tmp1 = __builtin_elementwise_copysign(f, f64);
+  //expected-error@-1 {{arguments are of different types ('float' vs 'double')}}
+
+  float4 v4f32 = 0.0f;
+  float4 tmp2 = __builtin_elementwise_copysign(v4f32, f);
+  // expected-error@-1 {{arguments are of different types ('float4' (vector of 4 'float' values) vs 'float')}}
+
+  float tmp3 = __builtin_elementwise_copysign(f, v4f32);
+  // expected-error@-1 {{arguments are of different types ('float' vs 'float4' (vector of 4 'float' values))}}
+
+  float2 v2f32 = 0.0f;
+  double4 v4f64 = 0.0;
+  double4 tmp4 = __builtin_elementwise_copysign(v4f64, v4f32);
+  // expected-error@-1 {{arguments are of different types ('double4' (vector of 4 'double' values) vs 'float4' (vector of 4 'float' values))}}
+
+  float4 tmp6 = __builtin_elementwise_copysign(v4f32, v4f64);
+  // expected-error@-1 {{arguments are of different types ('float4' (vector of 4 'float' values) vs 'double4' (vector of 4 'double' values))}}
+
+  float4 tmp7 = __builtin_elementwise_copysign(v4f32, v2f32);
+  // expected-error@-1 {{arguments are of different types ('float4' (vector of 4 'float' values) vs 'float2' (vector of 2 'float' values))}}
+
+  float2 tmp8 = __builtin_elementwise_copysign(v2f32, v4f32);
+  // expected-error@-1 {{arguments are of different types ('float2' (vector of 2 'float' values) vs 'float4' (vector of 4 'float' values))}}
+
+  float2 tmp9 = __builtin_elementwise_copysign(v4f32, v4f32);
+  // expected-error@-1 {{initializing 'float2' (vector of 2 'float' values) with an expression of incompatible type 'float4' (vector of 4 'float' values)}}
 }
