[clang] [HLSL] Implement the 'and' HLSL function (PR #127098)
Farzon Lotfi via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 16 18:44:35 PST 2025
================
@@ -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,
----------------
farzonl wrote:
> On this point I actually disagree. We should get this PR into a state where its diagnostics are at the high quality we want, then we can work back to the other diagnostics and correct them.
I’d prefer to take the time to plan a cohesive fix that updates all the tests rather than introducing an ad hoc solution for `and`, which would likely result in another ad hoc fix for the `or` builtin (also scheduled for this sprint).
For me, any work scheduled in the current sprint has a high bar for deviating from our established best practices. This is especially important because my primary goal is to ensure a smooth and predictable landing for the intrinsics workstream, as it plays a key role in onboarding new contributors.
Next week, I’d like to discuss a change that would either:
1. Consolidate `CheckArgTypeIsCorrect` and `CheckScalarOrVector`.
2. Modify `CheckArgTypeIsCorrect` to use the `err_typecheck_expect_scalar_or_vector` diagnostic and stop constructing the expected vector type. This would mean we stop printing an expected type with the syntactic sugar missing. Additionally, we would update ExpectedType to be a QualType vector or ArrayRef ExpectedTypes, allowing us to properly capture multiple overloads.
ie go from
```cpp
static bool CheckArgTypeIsCorrect(
Sema *S, Expr *Arg, QualType ExpectedType,
llvm::function_ref<bool(clang::QualType PassedType)> Check) {
QualType PassedType = Arg->getType();
if (Check(PassedType)) {
if (auto *VecTyA = PassedType->getAs<VectorType>())
ExpectedType = S->Context.getVectorType(
ExpectedType, VecTyA->getNumElements(), VecTyA->getVectorKind());
S->Diag(Arg->getBeginLoc(), diag::err_typecheck_convert_incompatible)
<< PassedType << ExpectedType << 1 << 0 << 0;
return true;
}
return false;
}
```
to
```cpp
static bool CheckArgTypeIsCorrect(
Sema *S, Expr *Arg, llvm::SmallVectorImpl<QualType> ExpectedTypes,
llvm::function_ref<bool(clang::QualType PassedType)> Check) {
QualType PassedType = Arg->getType();
if (Check(PassedType)) {
for (const clang::QualType &ExpectedType : ExpectedTypes)
S->Diag(TheCall->getArg(0)->getBeginLoc(),
diag::err_typecheck_expect_scalar_or_vector)
<< ArgType << ExpectedType;
```
My feeling is we can achieve what you want to do here but it would turn out better if these type of fixes had time to be designed outside of the PR process. I hope this better explains where I'm coming from.
https://github.com/llvm/llvm-project/pull/127098
More information about the cfe-commits
mailing list