================
@@ -2245,6 +2245,36 @@ bool SemaHLSL::CheckBuiltinFunctionCall(unsigned 
BuiltinID, CallExpr *TheCall) {
 
     break;
   }
+  case Builtin::BI__builtin_hlsl_and: {
+    if (SemaRef.checkArgCount(TheCall, 2))
+      return true;
+    if (CheckVectorElementCallArgs(&SemaRef, TheCall))
+      return true;
+
+    // CheckVectorElementCallArgs(...) guarantees both args are the same type.
+    assert(TheCall->getArg(0)->getType() == TheCall->getArg(1)->getType() &&
+           "Both args must be of the same type");
+
+    // check that the arguments are bools or, if vectors,
+    // vectors of bools
+    QualType ArgTy = TheCall->getArg(0)->getType();
+    if (const auto *VecTy = ArgTy->getAs<VectorType>()) {
+      ArgTy = VecTy->getElementType();
+    }
+    if (!getASTContext().hasSameUnqualifiedType(ArgTy,
----------------
llvm-beanz wrote:

> We know the expected types because we have defined them in sema per builtin. 
> Thats how `CheckAllArgsHaveFloatRepresentation`, 
> `CheckFloatOrHalfRepresentations`, `CheckUnsignedIntRepresentation` came to 
> be in the first place. The Expected types are specified in these sema rules. 
> and a spot check to `hlsl_intrinsics.h` should show they are the correct 
> expected types.

I think this is flawed logic. The source is ambiguous, we don't "know" the 
expected type we're guessing it.

>  What im proposing is that we pass the list of all the expected types instead 
> of just picking one.

This sounds like a recipe for throwing out a lot of errors and making it really 
hard for the user to process.

> `CheckFloatOrHalfRepresentations` currently used by `cross` is a wrapper for 
> `CheckArgTypeIsCorrect`. The problem you raised about alerting on `float` and 
> not`half` is caused because we are only passing one expected type to 
> CheckArgTypeIsCorrect. Changing this to a list allows us to do a diangostic 
> per expected type.

I don't think this solves the problem, I think it just makes us spew more 
errors at the user forcing them to decode a stream of messages.
 
> Further switching to using `CheckScalarOrVector` has the same problem 
> switching to it in the cross case would force you to do an error diagnostic 
> on a "arbitrarily expected" type.

I'm not suggesting we switch cross to exactly `CheckScalarOrVector` as it is. 
I'm stating that our current pattern is not providing good clear diagnostics to 
our users, and we need to reconsider it. For `cross` we should be able to emit 
a diagnostic that the arguments must be "16-bit or 32-bit floating point or 
vectors of such types", which would be much clearer and more concise than a 
spew of "expected float" and "expected half" diagnostics.

**For the issue in this PR:** `CheckScalarOrVector` does _exactly_ what we 
want. It allows printing an error message that the arguments must be "`bool` or 
vector of such type", which exactly describes the expected usage.



https://github.com/llvm/llvm-project/pull/127098
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to