Skip to content

[clang][sema] consolidate diags for incompatible_vector_* #83609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 2, 2024

Conversation

farzonl
Copy link
Member

@farzonl farzonl commented Mar 1, 2024

removing the additions of err_vec_builtin_non_vector_all & err_vec_builtin_incompatible_vector_all
caused by #83077.

Instead adding a select option to err_vec_builtin_non_vector & err_vec_builtin_incompatible_vector to account for uses where there are more than two arguments.

@farzonl farzonl requested a review from llvm-beanz March 1, 2024 19:26
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Mar 1, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-hlsl

Author: Farzon Lotfi (farzonl)

Changes

removing the additions of err_vec_builtin_non_vector_all & err_vec_builtin_incompatible_vector_all
caused by #83077.

Instead adding a select option to err_vec_builtin_non_vector & err_vec_builtin_incompatible_vector to account for uses where there are more than two arguments.


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

4 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2-7)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+11-9)
  • (modified) clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl (+14-14)
  • (modified) clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl (+1-1)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 938de5859513f8..632b945c345098 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10266,15 +10266,10 @@ def err_block_on_vm : Error<
 def err_sizeless_nonlocal : Error<
   "non-local variable with sizeless type %0">;
 
-def err_vec_builtin_non_vector_all : Error<
- "all arguments to %0 must be vectors">;
-def err_vec_builtin_incompatible_vector_all : Error<
-  "all arguments to %0 must have vectors of the same type">;
-
 def err_vec_builtin_non_vector : Error<
- "first two arguments to %0 must be vectors">;
+ "%select{first two|all}1 arguments to %0 must be vectors">;
 def err_vec_builtin_incompatible_vector : Error<
-  "first two arguments to %0 must have the same type">;
+  "%select{first two|all}1 arguments to %0 must have the same type">;
 def err_vsx_builtin_nonconstant_argument : Error<
   "argument %0 to %1 must be a 2-bit unsigned literal (i.e. 0, 1, 2 or 3)">;
 
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 9f9b0a0baba666..4d190fab560bcb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5218,15 +5218,15 @@ bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
         // Note: type promotion is intended to be handeled via the intrinsics
         //  and not the builtin itself.
         S->Diag(TheCall->getBeginLoc(),
-                diag::err_vec_builtin_incompatible_vector_all)
-            << TheCall->getDirectCallee()
+                diag::err_vec_builtin_incompatible_vector)
+            << TheCall->getDirectCallee() << (TheCall->getNumArgs() != 2)
             << SourceRange(A.get()->getBeginLoc(), B.get()->getEndLoc());
         retValue = true;
       }
       if (VecTyA->getNumElements() != VecTyB->getNumElements()) {
         // if we get here a HLSLVectorTruncation is needed.
-        S->Diag(BuiltinLoc, diag::err_vec_builtin_incompatible_vector_all)
-            << TheCall->getDirectCallee()
+        S->Diag(BuiltinLoc, diag::err_vec_builtin_incompatible_vector)
+            << TheCall->getDirectCallee() << (TheCall->getNumArgs() != 2)
             << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                            TheCall->getArg(1)->getEndLoc());
         retValue = true;
@@ -5241,8 +5241,8 @@ bool CheckVectorElementCallArgs(Sema *S, CallExpr *TheCall) {
 
   // Note: if we get here one of the args is a scalar which
   // requires a VectorSplat on Arg0 or Arg1
-  S->Diag(BuiltinLoc, diag::err_vec_builtin_non_vector_all)
-      << TheCall->getDirectCallee()
+  S->Diag(BuiltinLoc, diag::err_vec_builtin_non_vector)
+      << TheCall->getDirectCallee() << (TheCall->getNumArgs() != 2)
       << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                      TheCall->getArg(1)->getEndLoc());
   return true;
@@ -9472,7 +9472,7 @@ bool Sema::SemaBuiltinVSX(CallExpr *TheCall) {
   if ((!Arg1Ty->isVectorType() && !Arg1Ty->isDependentType()) ||
       (!Arg2Ty->isVectorType() && !Arg2Ty->isDependentType())) {
     return Diag(BuiltinLoc, diag::err_vec_builtin_non_vector)
-           << TheCall->getDirectCallee()
+           << TheCall->getDirectCallee() << /*isMorethantwoArgs*/ false
            << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                           TheCall->getArg(1)->getEndLoc());
   }
@@ -9480,7 +9480,7 @@ bool Sema::SemaBuiltinVSX(CallExpr *TheCall) {
   // Check the first two arguments are the same type.
   if (!Context.hasSameUnqualifiedType(Arg1Ty, Arg2Ty)) {
     return Diag(BuiltinLoc, diag::err_vec_builtin_incompatible_vector)
-           << TheCall->getDirectCallee()
+           << TheCall->getDirectCallee() << /*isMorethantwoArgs*/ false
            << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                           TheCall->getArg(1)->getEndLoc());
   }
@@ -9516,7 +9516,7 @@ ExprResult Sema::SemaBuiltinShuffleVector(CallExpr *TheCall) {
     if (!LHSType->isVectorType() || !RHSType->isVectorType())
       return ExprError(
           Diag(TheCall->getBeginLoc(), diag::err_vec_builtin_non_vector)
-          << TheCall->getDirectCallee()
+          << TheCall->getDirectCallee() << /*isMorethantwoArgs*/ false
           << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                          TheCall->getArg(1)->getEndLoc()));
 
@@ -9532,12 +9532,14 @@ ExprResult Sema::SemaBuiltinShuffleVector(CallExpr *TheCall) {
         return ExprError(Diag(TheCall->getBeginLoc(),
                               diag::err_vec_builtin_incompatible_vector)
                          << TheCall->getDirectCallee()
+                         << /*isMorethantwoArgs*/ false
                          << SourceRange(TheCall->getArg(1)->getBeginLoc(),
                                         TheCall->getArg(1)->getEndLoc()));
     } else if (!Context.hasSameUnqualifiedType(LHSType, RHSType)) {
       return ExprError(Diag(TheCall->getBeginLoc(),
                             diag::err_vec_builtin_incompatible_vector)
                        << TheCall->getDirectCallee()
+                       << /*isMorethantwoArgs*/ false
                        << SourceRange(TheCall->getArg(0)->getBeginLoc(),
                                       TheCall->getArg(1)->getEndLoc()));
     } else if (numElements != numResElements) {
diff --git a/clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl
index 8de8f86d7eb260..d4437e47b06e34 100644
--- a/clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/dot-errors.hlsl
@@ -22,7 +22,7 @@ float test_dot_vector_size_mismatch(float3 p0, float2 p1) {
 
 float test_dot_builtin_vector_size_mismatch(float3 p0, float2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 float test_dot_scalar_mismatch(float p0, int p1) {
@@ -38,70 +38,70 @@ float test_dot_element_type_mismatch(int2 p0, float2 p1) {
 //NOTE: for all the *_promotion we are intentionally not handling type promotion in builtins
 float test_builtin_dot_vec_int_to_float_promotion(int2 p0, float2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 int64_t test_builtin_dot_vec_int_to_int64_promotion(int64_t2 p0, int2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 float test_builtin_dot_vec_half_to_float_promotion(float2 p0, half2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 #ifdef __HLSL_ENABLE_16_BIT
 float test_builtin_dot_vec_int16_to_float_promotion(float2 p0, int16_t2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 half test_builtin_dot_vec_int16_to_half_promotion(half2 p0, int16_t2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 int test_builtin_dot_vec_int16_to_int_promotion(int2 p0, int16_t2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 
 int64_t test_builtin_dot_vec_int16_to_int64_promotion(int64_t2 p0,
                                                       int16_t2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must have vectors of the same type}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must have the same type}}
 }
 #endif
 
 float test_builtin_dot_float2_splat(float p0, float2 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 float test_builtin_dot_float3_splat(float p0, float3 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 float test_builtin_dot_float4_splat(float p0, float4 p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 float test_dot_float2_int_splat(float2 p0, int p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 float test_dot_float3_int_splat(float3 p0, int p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 float test_builtin_dot_int_vect_to_float_vec_promotion(int2 p0, float p1) {
   return __builtin_hlsl_dot(p0, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_dot' must be vectors}}
+  // expected-error@-1 {{first two arguments to '__builtin_hlsl_dot' must be vectors}}
 }
 
 int test_builtin_dot_bool_type_promotion(bool p0, bool p1) {
diff --git a/clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl
index 4ec5a4cdd26a30..f6ce87e7c33e3e 100644
--- a/clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/lerp-errors.hlsl
@@ -27,7 +27,7 @@ float2 test_lerp_vector_size_mismatch(float3 p0, float2 p1) {
 
 float2 test_lerp_builtin_vector_size_mismatch(float3 p0, float2 p1) {
   return __builtin_hlsl_lerp(p0, p1, p1);
-  // expected-error@-1 {{all arguments to '__builtin_hlsl_lerp' must have vectors of the same type}}
+  // expected-error@-1 {{all arguments to '__builtin_hlsl_lerp' must have the same type}}
 }
 
 float test_lerp_scalar_mismatch(float p0, half p1) {

@farzonl farzonl force-pushed the stylefix/replace-diag-with-select branch from 99a04a8 to bfd8afd Compare March 1, 2024 20:31
Copy link
Contributor

@pow2clk pow2clk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nitpicks. I also find it much harder to rereview a change when a force push has gone through. I don't know how things are done around here, but if it can be avoided, I prefer it.

@farzonl
Copy link
Member Author

farzonl commented Mar 1, 2024

@pow2clk force doesn't seem necessary b\c squash merge is the default behavior for this project. I did it because i tend to rebase my branches when I put out revisions. I'll stop doing that.

@farzonl farzonl merged commit 69b8372 into llvm:main Mar 2, 2024
@farzonl farzonl deleted the stylefix/replace-diag-with-select branch March 2, 2024 00:11
@farzonl farzonl self-assigned this Nov 2, 2024
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

5 participants