================ @@ -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