-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[HLSL] Convert vectors to bool for unary ! #163857
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
Conversation
HLSL extends C++'s requirement that unary `!` apply to boolean arguments and produces boolean results to apply to vectors. This change implements implicit conversion for non-boolean vector operands to boolean vectors. I've noted this behavior in the issue tracking writing the HLSL specification section on unary operators (microsoft/hlsl-specs#686). Fixes llvm#162913
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Chris B (llvm-beanz) ChangesHLSL extends C++'s requirement that unary I've noted this behavior in the issue tracking writing the HLSL specification section on unary operators Fixes #162913 Full diff: https://github.com/llvm/llvm-project/pull/163857.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3e0e9bb5e39e5..e42d30cb5acc8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15944,6 +15944,20 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
<< resultType << Input.get()->getSourceRange());
}
+ } else if (Context.getLangOpts().HLSL && resultType->isVectorType() &&
+ !resultType->hasBooleanRepresentation()) {
+ // HLSL unary logical not behaves like C++, which states that the
+ // operand is onverted to bool and the result is bool, however HLSL
+ // extends this property to vectors.
+ const VectorType *VTy = resultType->castAs<VectorType>();
+ resultType =
+ Context.getExtVectorType(Context.BoolTy, VTy->getNumElements());
+
+ Input = ImpCastExprToType(
+ Input.get(), resultType,
+ ScalarTypeToBooleanCastKind(VTy->getElementType()))
+ .get();
+ break;
} else if (resultType->isExtVectorType()) {
if (Context.getLangOpts().OpenCL &&
Context.getLangOpts().getOpenCLCompatibleVersion() < 120) {
diff --git a/clang/test/CodeGenHLSL/Operators/logical-not.hlsl b/clang/test/CodeGenHLSL/Operators/logical-not.hlsl
new file mode 100644
index 0000000000000..0f9d0677d8610
--- /dev/null
+++ b/clang/test/CodeGenHLSL/Operators/logical-not.hlsl
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-library -disable-llvm-passes -emit-llvm -finclude-default-header -fnative-half-type -o - %s | FileCheck %s
+
+// CHECK-LABEL: case1
+// CHECK: [[ToBool:%.*]] = icmp ne <2 x i32> {{.*}}, zeroinitializer
+// CHECK-NEXT: [[BoolCmp:%.*]] = icmp eq <2 x i1> [[ToBool]], zeroinitializer
+// CHECK-NEXT: {{.*}} = zext <2 x i1> [[BoolCmp]] to <2 x i32>
+export uint32_t2 case1(uint32_t2 b) {
+ return !b;
+}
+
+// CHECK-LABEL: case2
+// CHECK: [[ToBool:%.*]] = icmp ne <3 x i32> {{.*}}, zeroinitializer
+// CHECK-NEXT: [[BoolCmp:%.*]] = icmp eq <3 x i1> [[ToBool]], zeroinitializer
+// CHECK-NEXT: {{.*}} = zext <3 x i1> [[BoolCmp]] to <3 x i32>
+export int32_t3 case2(int32_t3 b) {
+ return !b;
+}
+
+// CHECK-LABEL: case3
+// CHECK: [[ToBool:%.*]] = fcmp reassoc nnan ninf nsz arcp afn une half {{.*}}, 0xH0000
+// CHECK-NEXT: [[BoolCmp:%.*]] = xor i1 [[ToBool]], true
+// CHECK-NEXT: {{.*}} = uitofp i1 [[BoolCmp]] to half
+export float16_t case3(float16_t b) {
+ return !b;
+}
+
+// CHECK-LABEL: case4
+// CHECK: [[ToBool:%.*]] = fcmp reassoc nnan ninf nsz arcp afn une <4 x float> {{.*}}, zeroinitializer
+// CHECK-NEXT: [[BoolCmp:%.*]] = icmp eq <4 x i1> [[ToBool]], zeroinitializer
+// CHECK-NEXT: {{.*}} = uitofp <4 x i1> [[BoolCmp]] to <4 x float>
+export float4 case4(float4 b) {
+ return !b;
+}
diff --git a/clang/test/SemaHLSL/Operators/logical-not.hlsl b/clang/test/SemaHLSL/Operators/logical-not.hlsl
new file mode 100644
index 0000000000000..d06ca3982be05
--- /dev/null
+++ b/clang/test/SemaHLSL/Operators/logical-not.hlsl
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -ast-dump -ast-dump-filter=case | FileCheck %s
+
+// CHECK-LABEL: FunctionDecl {{.*}} used case1 'uint32_t2 (uint32_t2)'
+// CHECK-NEXT: ParmVarDecl {{.*}} used b 'uint32_t2':'vector<uint32_t, 2>'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<uint32_t, 2>' <IntegralCast>
+// CHECK-NEXT: UnaryOperator {{.*}} 'vector<bool, 2>' prefix '!' cannot overflow
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<bool, 2>' <IntegralToBoolean>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'uint32_t2':'vector<uint32_t, 2>' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'uint32_t2':'vector<uint32_t, 2>' lvalue ParmVar {{.*}} 'b' 'uint32_t2':'vector<uint32_t, 2>'
+export uint32_t2 case1(uint32_t2 b) {
+ return !b;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} used case2 'int32_t3 (int32_t3)'
+// CHECK-NEXT: ParmVarDecl {{.*}} used b 'int32_t3':'vector<int32_t, 3>'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<int32_t, 3>' <IntegralCast>
+// CHECK-NEXT: UnaryOperator {{.*}} 'vector<bool, 3>' prefix '!' cannot overflow
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<bool, 3>' <IntegralToBoolean>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'int32_t3':'vector<int32_t, 3>' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'int32_t3':'vector<int32_t, 3>' lvalue ParmVar {{.*}} 'b' 'int32_t3':'vector<int32_t, 3>'
+export int32_t3 case2(int32_t3 b) {
+ return !b;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} used case3 'float16_t (float16_t)'
+// CHECK-NEXT: ParmVarDecl {{.*}} used b 'float16_t':'half'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float16_t':'half' <IntegralToFloating>
+// CHECK-NEXT: UnaryOperator {{.*}} 'bool' prefix '!' cannot overflow
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'bool' <FloatingToBoolean>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float16_t':'half' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'float16_t':'half' lvalue ParmVar {{.*}} 'b' 'float16_t':'half'
+export float16_t case3(float16_t b) {
+ return !b;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} used case4 'float4 (float4)'
+// CHECK-NEXT: ParmVarDecl {{.*}} used b 'float4':'vector<float, 4>'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT: ReturnStmt
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<float, 4>' <IntegralToFloating>
+// CHECK-NEXT: UnaryOperator {{.*}} 'vector<bool, 4>' prefix '!' cannot overflow
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'vector<bool, 4>' <FloatingToBoolean>
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'float4':'vector<float, 4>' <LValueToRValue>
+// CHECK-NEXT: DeclRefExpr {{.*}} 'float4':'vector<float, 4>' lvalue ParmVar {{.*}} 'b' 'float4':'vector<float, 4>'
+export float4 case4(float4 b) {
+ return !b;
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I tested a few of the affected DML operator tests with this patch and they passed!
I'm surprised that this is HLSL specific, though it obviously falls into the minutiae of a few different language extensions here. Is the behaviour in C++ mode here intentional, or did it just follow the opencl behaviour from ext_vector? The change that introduced logical not for vector isn't very clear about this, but may be matching GCC, I'm not sure: https://reviews.llvm.org/D80979. In any case, I think we should probably investigate whether we're doing the "right" thing in C++ mode separately from the change itself. |
@erichkeane might be the right person to weigh in on the C/C++ side, but my guess is that C++ does what it currently does intentionally for compatibility with SSE where boolean true in vectors is expected to be all bits on (so sign-extending i1 gets the intended result). |
Co-authored-by: Farzon Lotfi <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely do similar stuff for our ExtVector type, so I don't think of this as horribly inconsistent with the language. However, the base-vector
type is 'owned' by GCC (Clang's version is the ExtVectorType), so I don't think we should be modifying the GCC version unless they are going to do the same. At that point, I don't think it should be HLSL specific.
That said, extending this 'splat-esque' behavior for ExtVectorType to HLSL should be non-controversial.
Co-authored-by: Erich Keane <[email protected]>
HLSL extends C++'s requirement that unary
!
apply to boolean arguments and produces boolean results to apply to vectors. This change implements implicit conversion for non-boolean vector operands to boolean vectors.I've noted this behavior in the issue tracking writing the HLSL specification section on unary operators
(microsoft/hlsl-specs#686).
Fixes #162913