[HLSL] Make it possible to assign an array from a cbuffer #134174
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-hlsl Author: Sarah Spall (spall) ChangesUpdate Sema Checking to always do an HLSL Array RValue cast in the case we are dealing with hlsl constant array types Full diff: https://github.com/llvm/llvm-project/pull/134174.diff 3 Files Affected:
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index fa492bc124abd..dd4887c0b2878 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4418,19 +4418,13 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
case ICK_HLSL_Array_RValue:
if (ToType->isArrayParameterType()) {
FromType = Context.getArrayParameterType(FromType);
- From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
- /*BasePath=*/nullptr, CCK)
- .get();
- } else { // FromType must be ArrayParameterType
- assert(FromType->isArrayParameterType() &&
- "FromType must be ArrayParameterType in ICK_HLSL_Array_RValue \
- if it is not ToType");
+ } else if (FromType->isArrayParameterType()) {
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
FromType = APT->getConstantArrayType(Context);
- From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
- /*BasePath=*/nullptr, CCK)
- .get();
}
+ From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
+ /*BasePath=*/nullptr, CCK)
+ .get();
break;
case ICK_Function_To_Pointer:
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 1802f8f4e1f91..d282fe50e49f1 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2268,17 +2268,15 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
// handling here.
if (ToType->isArrayParameterType()) {
FromType = S.Context.getArrayParameterType(FromType);
- SCS.First = ICK_HLSL_Array_RValue;
} else if (FromType->isArrayParameterType()) {
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
FromType = APT->getConstantArrayType(S.Context);
- SCS.First = ICK_HLSL_Array_RValue;
- } else {
- SCS.First = ICK_Identity;
}
- if (S.Context.getCanonicalType(FromType) !=
- S.Context.getCanonicalType(ToType))
+ SCS.First = ICK_HLSL_Array_RValue;
+
+ if (FromType.getCanonicalType().getUnqualifiedType() !=
+ ToType.getCanonicalType().getUnqualifiedType())
return false;
SCS.setAllToTypes(ToType);
diff --git a/clang/test/CodeGenHLSL/ArrayAssignable.hlsl b/clang/test/CodeGenHLSL/ArrayAssignable.hlsl
index e2ff2de68ed99..723d521596baf 100644
--- a/clang/test/CodeGenHLSL/ArrayAssignable.hlsl
+++ b/clang/test/CodeGenHLSL/ArrayAssignable.hlsl
@@ -1,4 +1,23 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s --enable-var-scope
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -finclude-default-header -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+
+struct S {
+ int x;
+ float f;
+};
+
+// CHECK: [[CBLayout:%.*]] = type <{ [2 x float], [2 x <4 x i32>], [2 x [2 x i32]], [1 x target("dx.Layout", %S, 8, 0, 4)] }>
+// CHECK: @CBArrays.cb = global target("dx.CBuffer", target("dx.Layout", [[CBLayout]], 136, 0, 32, 64, 128))
+// CHECK: @c1 = external addrspace(2) global [2 x float], align 4
+// CHECK: @c2 = external addrspace(2) global [2 x <4 x i32>], align 16
+// CHECK: @c3 = external addrspace(2) global [2 x [2 x i32]], align 4
+// CHECK: @c4 = external addrspace(2) global [1 x target("dx.Layout", %S, 8, 0, 4)], align 4
+
+cbuffer CBArrays {
+ float c1[2];
+ int4 c2[2];
+ int c3[2][2];
+ S c4[1];
+}
// CHECK-LABEL: define void {{.*}}arr_assign1
// CHECK: [[Arr:%.*]] = alloca [2 x i32], align 4
@@ -116,3 +135,45 @@ void arr_assign7() {
int Arr2[2][2] = {{0, 0}, {1, 1}};
(Arr = Arr2)[0] = {6, 6};
}
+
+// Verify you can assign from a cbuffer array
+
+// CHECK-LABEL: define void {{.*}}arr_assign8
+// CHECK: [[C:%.*]] = alloca [2 x float], align 4
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[C]], ptr align 4 {{.*}}, i32 8, i1 false)
+// CHECK-NEXT: call void @llvm.memcpy.p0.p2.i32(ptr align 4 [[C]], ptr addrspace(2) align 4 @c1, i32 8, i1 false)
+// CHECK-NEXT: ret void
+void arr_assign8() {
+ float C[2] = {1.0, 2.0};
+ C = c1;
+}
+
+// CHECK-LABEL: define void {{.*}}arr_assign9
+// CHECK: [[C:%.*]] = alloca [2 x <4 x i32>], align 16
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 16 [[C]], ptr align 16 {{.*}}, i32 32, i1 false)
+// CHECK-NEXT: call void @llvm.memcpy.p0.p2.i32(ptr align 16 [[C]], ptr addrspace(2) align 16 @c2, i32 32, i1 false)
+// CHECK-NEXT: ret void
+void arr_assign9() {
+ int4 C[2] = {1,2,3,4,5,6,7,8};
+ C = c2;
+}
+
+// CHECK-LABEL: define void {{.*}}arr_assign10
+// CHECK: [[C:%.*]] = alloca [2 x [2 x i32]], align 4
+// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[C]], ptr align 4 {{.*}}, i32 16, i1 false)
+// CHECK-NEXT: call void @llvm.memcpy.p0.p2.i32(ptr align 4 [[C]], ptr addrspace(2) align 4 @c3, i32 16, i1 false)
+// CHECK-NEXT: ret void
+void arr_assign10() {
+ int C[2][2] = {1,2,3,4};
+ C = c3;
+}
+
+// CHECK-LABEL: define void {{.*}}arr_assign11
+// CHECK: [[C:%.*]] = alloca [1 x %struct.S], align 4
+// CHECK: call void @llvm.memcpy.p0.p2.i32(ptr align 4 [[C]], ptr addrspace(2) align 4 @c4, i32 8, i1 false)
+// CHECK-NEXT: ret void
+void arr_assign11() {
+ S s = {1, 2.0};
+ S C[1] = {s};
+ C = c4;
+}
|
inbelic
approved these changes
Apr 8, 2025
farzonl
approved these changes
Apr 9, 2025
bogner
approved these changes
Apr 18, 2025
There was a problem hiding this comment.
The fact that we get memcpy here points out some of the issues we have with representing cbuffers - it would really be nicer if this ended up generating elementwise copies instead. However, I think fixing that is out of the scope of this change (it'll also come up for structs, etc) and will be rather complicated, so this looks reasonable to me.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/23242 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/21637 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/16372 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/3846 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/18685 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/13451 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/29445 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/23949 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/25265 Here is the relevant piece of the build log for the reference |
|
Hi, this turned a few bots red. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/55/builds/10195 Here is the relevant piece of the build log for the reference |
It can be reverted |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/85/builds/7901 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/24/builds/7603 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update Sema Checking to always do an HLSL Array RValue cast in the case we are dealing with hlsl constant array types
Instead of comparing canonical types, compare canonical unqualified types
Add a test to show it is possible to assign an array from a cbuffer.
Closes #133767