[clang] Update typechecking of builtin elementwise ternary math operators

[ckoparkar]

Fixes #155405

For scalars we directly compare their unqualified types. But even if we compare unqualified vector types, a difference in qualifiers in the element types can make the vector types be considered not equal. For example, vector of 4 'const float' values vs vector of 4 'float' values. So we compare unqualified types of their elements and number of elements.

/cc @tbaederr

[llvmbot]

@llvm/pr-subscribers-clang

Author: Chaitanya Koparkar (ckoparkar)

Changes

Fixes #155405

For scalars we directly compare their unqualified types. But even if we compare unqualified vector types, a difference in qualifiers in the element types can make the vector types be considered not equal. For example, vector of 4 'const float' values vs vector of 4 'float' values. So we compare unqualified types of their elements and number of elements.

/cc @tbaederr

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaChecking.cpp (+57-12)
• (modified) clang/test/Sema/builtins-elementwise-math.c (+19-2)

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 908ac31c6ac98..8ce308fd120e8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15884,6 +15884,58 @@ static bool checkBuiltinVectorMathMixedEnums(Sema &S, Expr *LHS, Expr *RHS,
   return false;
 }
 
+/// Check if all arguments have the same type. If the types don't match, emit an
+/// error message and return true. Otherwise return false.
+///
+/// For scalars we directly compare their unqualified types. But even if we
+/// compare unqualified vector types, a difference in qualifiers in the element
+/// types can make the vector types be considered not equal. For example,
+/// vector of 4 'const float' values vs vector of 4 'float' values.
+/// So we compare unqualified types of their elements and number of elements.
+/// See GitHub issue #155405.
+static bool checkBuiltinVectorMathArgTypes(Sema &SemaRef, unsigned NumArgs,
+                                           Expr *Args[]) {
+  assert(NumArgs > 0 && "Should have at least one argument.");
+
+  auto EmitError = [&](int I) {
+    SemaRef.Diag(Args[0]->getBeginLoc(),
+                 diag::err_typecheck_call_different_arg_types)
+        << Args[0]->getType() << Args[I]->getType();
+  };
+
+  QualType Ty0 = Args[0]->getType();
+
+  // Compare scalar types.
+  if (!Ty0->isVectorType()) {
+    for (unsigned I = 1; I < NumArgs; ++I)
+      if (!SemaRef.Context.hasSameUnqualifiedType(Ty0, Args[I]->getType())) {
+        EmitError(I);
+        return true;
+      }
+
+    return false;
+  }
+
+  // Compare vector types.
+  const auto *Vec0 = Ty0->castAs<VectorType>();
+  for (unsigned I = 1; I < NumArgs; ++I) {
+    QualType TyI = Args[I]->getType();
+    if (!TyI->isVectorType()) {
+      EmitError(I);
+      return true;
+    }
+    const auto *VecI = TyI->castAs<VectorType>();
+    if (!SemaRef.Context.hasSameUnqualifiedType(Vec0->getElementType(),
+                                                VecI->getElementType()) ||
+        Vec0->getNumElements() != VecI->getNumElements()) {
+      EmitError(I);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 std::optional<QualType>
 Sema::BuiltinVectorMath(CallExpr *TheCall,
                         EltwiseBuiltinArgTyRestriction ArgTyRestr) {
@@ -15905,13 +15957,11 @@ Sema::BuiltinVectorMath(CallExpr *TheCall,
 
   SourceLocation LocA = Args[0]->getBeginLoc();
   QualType TyA = Args[0]->getType();
-  QualType TyB = Args[1]->getType();
 
   if (checkMathBuiltinElementType(*this, LocA, TyA, ArgTyRestr, 1))
     return std::nullopt;
 
-  if (!Context.hasSameUnqualifiedType(TyA, TyB)) {
-    Diag(LocA, diag::err_typecheck_call_different_arg_types) << TyA << TyB;
+  if (checkBuiltinVectorMathArgTypes(*this, 2, Args)) {
     return std::nullopt;
   }
 
@@ -15948,17 +15998,12 @@ bool Sema::BuiltinElementwiseTernaryMath(
       return true;
   }
 
-  TheCall->setArg(0, Args[0]);
-  for (int I = 1; I < 3; ++I) {
-    if (Args[0]->getType().getCanonicalType() !=
-        Args[I]->getType().getCanonicalType()) {
-      return Diag(Args[0]->getBeginLoc(),
-                  diag::err_typecheck_call_different_arg_types)
-             << Args[0]->getType() << Args[I]->getType();
-    }
+  if (checkBuiltinVectorMathArgTypes(*this, 3, Args)) {
+    return true;
+  }
 
+  for (int I = 0; I < 3; ++I)
     TheCall->setArg(I, Args[I]);
-  }
 
   TheCall->setType(Args[0]->getType());
   return false;
diff --git a/clang/test/Sema/builtins-elementwise-math.c b/clang/test/Sema/builtins-elementwise-math.c
index 02dd44211e96a..d6ce5c0a34746 100644
--- a/clang/test/Sema/builtins-elementwise-math.c
+++ b/clang/test/Sema/builtins-elementwise-math.c
@@ -5,6 +5,7 @@ typedef double double4 __attribute__((ext_vector_type(4)));
 typedef float float2 __attribute__((ext_vector_type(2)));
 typedef float float3 __attribute__((ext_vector_type(3)));
 typedef float float4 __attribute__((ext_vector_type(4)));
+typedef const float cfloat4 __attribute__((ext_vector_type(4)));
 
 typedef int int2 __attribute__((ext_vector_type(2)));
 typedef int int3 __attribute__((ext_vector_type(3)));
@@ -1330,16 +1331,32 @@ void test_builtin_elementwise_fsh(int i32, int2 v2i32, short i16, int3 v3i32,
     // expected-error@-1 {{arguments are of different types ('int3' (vector of 3 'int' values) vs 'int2' (vector of 2 'int' values))}}
 }
 
+// Tests corresponding to GitHub issues #141397 and #155405.
+// Type mismatch error when 'builtin-elementwise-math' arguments have
+// different qualifiers, this should be well-formed.
 typedef struct {
   float3 b;
 } struct_float3;
-// This example uncovered a bug #141397 :
-// Type mismatch error when 'builtin-elementwise-math' arguments have different qualifiers, this should be well-formed
+
 float3 foo(float3 a,const struct_float3* hi) {
   float3 b = __builtin_elementwise_max((float3)(0.0f), a);
   return __builtin_elementwise_pow(b, hi->b.yyy);
 }
 
+float3 baz(float3 a, const struct_float3* hi) {
+  return __builtin_elementwise_fma(a, a, hi->b);
+}
+
+cfloat4 qux(cfloat4 x, float4 y, float4 z) {
+  float a = __builtin_elementwise_fma(x[0],y[0],z[0]);
+  return __builtin_elementwise_fma(x,y,z);
+}
+
+cfloat4 quux(cfloat4 x, float4 y) {
+  float a = __builtin_elementwise_pow(x[0],y[0]);
+  return __builtin_elementwise_pow(x,y);
+}
+
 void test_builtin_elementwise_ctlz(int i32, int2 v2i32, short i16,
                                    double f64, double2 v2f64) {
   f64 = __builtin_elementwise_ctlz(f64);

[ckoparkar]

• This patch only changes the behavior for input arguments. For consistency, we should change how canonical types are computed for these vectors, but that will have a widespread impact.

[Sirraide]

This still needs a release note.

[tbaederr]

Suggested change

	assert(Args.size() > 0 && "Should have at least one argument.");
	assert(!Args.empty() && "Should have at least one argument.");