Skip to content

[HLSL] Reland; Make it possible to assign an array from a cbuffer #136580

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

Merged
merged 4 commits into from
Apr 21, 2025

Conversation

spall
Copy link
Contributor

@spall spall commented Apr 21, 2025

Reland #134174
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

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" HLSL HLSL Language Support labels Apr 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

@llvm/pr-subscribers-hlsl

Author: Sarah Spall (spall)

Changes

Reland #134174
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


Full diff: https://github.com/llvm/llvm-project/pull/136580.diff

3 Files Affected:

  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+4-10)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+5-6)
  • (modified) clang/test/CodeGenHLSL/ArrayAssignable.hlsl (+62-1)
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d46322d75721e..f5a10e0db85ad 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4745,19 +4745,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 e4ff8c5489df3..5b224b6c08fef 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2275,17 +2275,16 @@ 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;
+
+    // Don't consider qualifiers, which include things like address spaces
+    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..6374f91230546 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 : register(b0) {
+  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;
+}

@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

@llvm/pr-subscribers-clang

Author: Sarah Spall (spall)

Changes

Reland #134174
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


Full diff: https://github.com/llvm/llvm-project/pull/136580.diff

3 Files Affected:

  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+4-10)
  • (modified) clang/lib/Sema/SemaOverload.cpp (+5-6)
  • (modified) clang/test/CodeGenHLSL/ArrayAssignable.hlsl (+62-1)
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d46322d75721e..f5a10e0db85ad 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -4745,19 +4745,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 e4ff8c5489df3..5b224b6c08fef 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -2275,17 +2275,16 @@ 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;
+
+    // Don't consider qualifiers, which include things like address spaces
+    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..6374f91230546 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 : register(b0) {
+  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;
+}

@spall spall merged commit f02b285 into llvm:main Apr 21, 2025
15 checks passed
@damyanp damyanp moved this to Closed in HLSL Support Apr 25, 2025
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…vm#136580)

Reland llvm#134174 
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 llvm#133767
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…vm#136580)

Reland llvm#134174 
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 llvm#133767
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…vm#136580)

Reland llvm#134174 
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 llvm#133767
@damyanp damyanp removed this from HLSL Support Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[HLSL] Assigning array from cbuffer fails
4 participants