-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[HLSL] Static resources fixes #166880
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
base: users/hekota/pr166844-res-internal
Are you sure you want to change the base?
[HLSL] Static resources fixes #166880
Conversation
- Enable assignment to static resource variables (fixes llvm#166458) - Initialize static resources and resource arrays with default constructor that sets the handle to poison
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-codegen Author: Helena Kotas (hekota) ChangesThis change fixes couple of issues with static resources:
Depends on #166844 Full diff: https://github.com/llvm/llvm-project/pull/166880.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index e392a12044a39..2cf601ca6a424 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -1025,12 +1025,13 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
ArraySubsExpr->getType()->isHLSLResourceRecordArray()) &&
"expected resource array subscript expression");
- // Let clang codegen handle local resource array subscripts,
+ // Let Clang codegen handle local and static resource array subscripts,
// or when the subscript references on opaque expression (as part of
// ArrayInitLoopExpr AST node).
const VarDecl *ArrayDecl =
dyn_cast_or_null<VarDecl>(getArrayDecl(ArraySubsExpr));
- if (!ArrayDecl || !ArrayDecl->hasGlobalStorage())
+ if (!ArrayDecl || !ArrayDecl->hasGlobalStorage() ||
+ ArrayDecl->getStorageClass() == SC_Static)
return std::nullopt;
// get the resource array type
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 0fea57b2e1799..b1256daafcdec 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5918,7 +5918,8 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
(D->getType()->isHLSLResourceRecord() ||
D->getType()->isHLSLResourceRecordArray())) {
Init = llvm::PoisonValue::get(getTypes().ConvertType(ASTTy));
- NeedsGlobalCtor = D->getType()->isHLSLResourceRecord();
+ NeedsGlobalCtor = D->getType()->isHLSLResourceRecord() ||
+ D->getStorageClass() == SC_Static;
} else if (D->hasAttr<LoaderUninitializedAttr>()) {
Init = llvm::UndefValue::get(getTypes().ConvertTypeForMem(ASTTy));
} else if (!InitExpr) {
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index e95fe16e6cb6c..1bcc074c080b2 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -3924,7 +3924,9 @@ void SemaHLSL::ActOnVariableDeclarator(VarDecl *VD) {
// process explicit bindings
processExplicitBindingsOnDecl(VD);
- if (VD->getType()->isHLSLResourceRecordArray()) {
+ // Add implicit binding attribute to non-static resource arrays.
+ if (VD->getType()->isHLSLResourceRecordArray() &&
+ VD->getStorageClass() != SC_Static) {
// If the resource array does not have an explicit binding attribute,
// create an implicit one. It will be used to transfer implicit binding
// order_ID to codegen.
@@ -4118,8 +4120,8 @@ bool SemaHLSL::ActOnUninitializedVarDecl(VarDecl *VD) {
if (VD->getType().getAddressSpace() == LangAS::hlsl_constant)
return true;
- // Initialize resources at the global scope
- if (VD->hasGlobalStorage()) {
+ // Initialize non-static resources at the global scope.
+ if (VD->hasGlobalStorage() && VD->getStorageClass() != SC_Static) {
const Type *Ty = VD->getType().getTypePtr();
if (Ty->isHLSLResourceRecord())
return initGlobalResourceDecl(VD);
@@ -4143,10 +4145,10 @@ bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr,
while (auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
E = ASE->getBase()->IgnoreParenImpCasts();
- // Report error if LHS is a resource declared at a global scope.
+ // Report error if LHS is a non-static resource declared at a global scope.
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
- if (VD->hasGlobalStorage()) {
+ if (VD->hasGlobalStorage() && VD->getStorageClass() != SC_Static) {
// assignment to global resource is not allowed
SemaRef.Diag(Loc, diag::err_hlsl_assign_to_global_resource) << VD;
SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) << VD;
diff --git a/clang/test/SemaHLSL/static_resources.hlsl b/clang/test/SemaHLSL/static_resources.hlsl
new file mode 100644
index 0000000000000..9997bdf6dca7b
--- /dev/null
+++ b/clang/test/SemaHLSL/static_resources.hlsl
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -emit-llvm -disable-llvm-passes -o - %s | llvm-cxxfilt | FileCheck %s
+
+// CHECK: [[ONE_STR:@.*]] = private unnamed_addr constant [4 x i8] c"One\00"
+// CHECK: [[ARRAY_STR:@.*]] = private unnamed_addr constant [6 x i8] c"Array\00"
+// CHECK-NOT: private unnamed_addr constant [{{[0-9]+}} x i8] c"Static
+
+RWBuffer<float> One : register(u1, space5);
+RWBuffer<float> Array[4][2] : register(u10, space6);
+
+// Check that the non-static resource One is initialized from binding on
+// startup (register 1, space 5).
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::__createFromBinding(unsigned int, unsigned int, int, unsigned int, char const*)
+// CHECK-SAME: (ptr {{.*}} @One, i32 noundef 1, i32 noundef 5, i32 noundef 1, i32 noundef 0, ptr noundef [[ONE_STR]])
+
+// Note that non-static resource arrays are not initialized on startup.
+// The individual resources from the array are initialized on access.
+
+static RWBuffer<float> StaticOne;
+static RWBuffer<float> StaticArray[2];
+
+// Check that StaticOne resource is initialized on startup with the default
+// constructor and not from binding. It will initalize the handle to poison.
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} @StaticOne)
+
+// Check that StaticArray elements are initialized on startup with the default
+// constructor and not from binding. The initializer will loop over the array
+// elements and call the default constructor for each one, setting the handle to poison.
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: br label %arrayctor.loop
+// CHECK: arrayctor.loop: ; preds = %arrayctor.loop, %entry
+// CHECK-NEXT: %arrayctor.cur = phi ptr [ @StaticArray, %entry ], [ %arrayctor.next, %arrayctor.loop ]
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} %arrayctor.cur)
+// CHECK-NEXT: %arrayctor.next = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %arrayctor.cur, i32 1
+// CHECK-NEXT: %arrayctor.done = icmp eq ptr %arrayctor.next, getelementptr inbounds (%"class.hlsl::RWBuffer", ptr @StaticArray, i32 2)
+// CHECK-NEXT: br i1 %arrayctor.done, label %arrayctor.cont, label %arrayctor.loop
+// CHECK: arrayctor.cont: ; preds = %arrayctor.loop
+// CHECK-NEXT: ret void
+
+// No other global initialization routines should be present.
+// CHECK-NOT: define internal void @__cxx_global_var_init{{.*}}
+
+[numthreads(4,1,1)]
+void main() {
+// CHECK: define internal void @main()()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %[[TMP0:.*]] = alloca %"class.hlsl::RWBuffer"
+
+ static RWBuffer<float> StaticLocal;
+// Check that StaticLocal is initialized to by default constructor to poison and not from binding
+// call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} @main()::StaticLocal)
+
+ StaticLocal = Array[2][0];
+// A[2][0] is accessed here, so it should be initialized from binding (register 10, space 6, index 4),
+// and then assigned to StaticLocal using = operator.
+// CHECK: call void @hlsl::RWBuffer<float>::__createFromBinding(unsigned int, unsigned int, int, unsigned int, char const*)
+// CHECK-SAME: (ptr {{.*}} %[[TMP0]], i32 noundef 10, i32 noundef 6, i32 noundef 8, i32 noundef 4, ptr noundef [[ARRAY_STR]])
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=({{.*}})(ptr {{.*}} @main()::StaticLocal, ptr {{.*}} %[[TMP0]])
+
+ StaticOne = One;
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=({{.*}})(ptr {{.*}} @StaticOne, ptr {{.*}} @One)
+
+ StaticArray[1] = One;
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=(hlsl::RWBuffer<float> const&)
+// CHECK-SAME: (ptr {{.*}} getelementptr inbounds ([2 x %"class.hlsl::RWBuffer"], ptr @StaticArray, i32 0, i32 1), ptr {{.*}} @One)
+
+ StaticLocal[0] = 123;
+// CHECK-NEXT: %[[PTR0:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)(ptr {{.*}} @main()::StaticLocal, i32 noundef 0)
+// CHECK-NEXT: store float 1.230000e+02, ptr %[[PTR0]]
+
+ StaticOne[1] = 456;
+// CHECK-NEXT: %[[PTR1:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)(ptr {{.*}}) @StaticOne, i32 noundef 1)
+// CHECK-NEXT: store float 4.560000e+02, ptr %[[PTR1]], align 4
+
+ StaticArray[1][2] = 789;
+// CHECK-NEXT: %[[PTR2:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)
+// CHECK-SAME: (ptr {{.*}} getelementptr inbounds ([2 x %"class.hlsl::RWBuffer"], ptr @StaticArray, i32 0, i32 1), i32 noundef 2)
+// CHECK-NEXT: store float 7.890000e+02, ptr %[[PTR2]], align 4
+}
+
+// No other binding initialization calls should be present.
+// CHECK-NOT: call void @hlsl::RWBuffer<float>::__createFrom{{.*}}Binding{{.*}}
|
|
@llvm/pr-subscribers-hlsl Author: Helena Kotas (hekota) ChangesThis change fixes couple of issues with static resources:
Depends on #166844 Full diff: https://github.com/llvm/llvm-project/pull/166880.diff 4 Files Affected:
diff --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index e392a12044a39..2cf601ca6a424 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -1025,12 +1025,13 @@ std::optional<LValue> CGHLSLRuntime::emitResourceArraySubscriptExpr(
ArraySubsExpr->getType()->isHLSLResourceRecordArray()) &&
"expected resource array subscript expression");
- // Let clang codegen handle local resource array subscripts,
+ // Let Clang codegen handle local and static resource array subscripts,
// or when the subscript references on opaque expression (as part of
// ArrayInitLoopExpr AST node).
const VarDecl *ArrayDecl =
dyn_cast_or_null<VarDecl>(getArrayDecl(ArraySubsExpr));
- if (!ArrayDecl || !ArrayDecl->hasGlobalStorage())
+ if (!ArrayDecl || !ArrayDecl->hasGlobalStorage() ||
+ ArrayDecl->getStorageClass() == SC_Static)
return std::nullopt;
// get the resource array type
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 0fea57b2e1799..b1256daafcdec 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5918,7 +5918,8 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
(D->getType()->isHLSLResourceRecord() ||
D->getType()->isHLSLResourceRecordArray())) {
Init = llvm::PoisonValue::get(getTypes().ConvertType(ASTTy));
- NeedsGlobalCtor = D->getType()->isHLSLResourceRecord();
+ NeedsGlobalCtor = D->getType()->isHLSLResourceRecord() ||
+ D->getStorageClass() == SC_Static;
} else if (D->hasAttr<LoaderUninitializedAttr>()) {
Init = llvm::UndefValue::get(getTypes().ConvertTypeForMem(ASTTy));
} else if (!InitExpr) {
diff --git a/clang/lib/Sema/SemaHLSL.cpp b/clang/lib/Sema/SemaHLSL.cpp
index e95fe16e6cb6c..1bcc074c080b2 100644
--- a/clang/lib/Sema/SemaHLSL.cpp
+++ b/clang/lib/Sema/SemaHLSL.cpp
@@ -3924,7 +3924,9 @@ void SemaHLSL::ActOnVariableDeclarator(VarDecl *VD) {
// process explicit bindings
processExplicitBindingsOnDecl(VD);
- if (VD->getType()->isHLSLResourceRecordArray()) {
+ // Add implicit binding attribute to non-static resource arrays.
+ if (VD->getType()->isHLSLResourceRecordArray() &&
+ VD->getStorageClass() != SC_Static) {
// If the resource array does not have an explicit binding attribute,
// create an implicit one. It will be used to transfer implicit binding
// order_ID to codegen.
@@ -4118,8 +4120,8 @@ bool SemaHLSL::ActOnUninitializedVarDecl(VarDecl *VD) {
if (VD->getType().getAddressSpace() == LangAS::hlsl_constant)
return true;
- // Initialize resources at the global scope
- if (VD->hasGlobalStorage()) {
+ // Initialize non-static resources at the global scope.
+ if (VD->hasGlobalStorage() && VD->getStorageClass() != SC_Static) {
const Type *Ty = VD->getType().getTypePtr();
if (Ty->isHLSLResourceRecord())
return initGlobalResourceDecl(VD);
@@ -4143,10 +4145,10 @@ bool SemaHLSL::CheckResourceBinOp(BinaryOperatorKind Opc, Expr *LHSExpr,
while (auto *ASE = dyn_cast<ArraySubscriptExpr>(E))
E = ASE->getBase()->IgnoreParenImpCasts();
- // Report error if LHS is a resource declared at a global scope.
+ // Report error if LHS is a non-static resource declared at a global scope.
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParens())) {
if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
- if (VD->hasGlobalStorage()) {
+ if (VD->hasGlobalStorage() && VD->getStorageClass() != SC_Static) {
// assignment to global resource is not allowed
SemaRef.Diag(Loc, diag::err_hlsl_assign_to_global_resource) << VD;
SemaRef.Diag(VD->getLocation(), diag::note_var_declared_here) << VD;
diff --git a/clang/test/SemaHLSL/static_resources.hlsl b/clang/test/SemaHLSL/static_resources.hlsl
new file mode 100644
index 0000000000000..9997bdf6dca7b
--- /dev/null
+++ b/clang/test/SemaHLSL/static_resources.hlsl
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.6-compute -emit-llvm -disable-llvm-passes -o - %s | llvm-cxxfilt | FileCheck %s
+
+// CHECK: [[ONE_STR:@.*]] = private unnamed_addr constant [4 x i8] c"One\00"
+// CHECK: [[ARRAY_STR:@.*]] = private unnamed_addr constant [6 x i8] c"Array\00"
+// CHECK-NOT: private unnamed_addr constant [{{[0-9]+}} x i8] c"Static
+
+RWBuffer<float> One : register(u1, space5);
+RWBuffer<float> Array[4][2] : register(u10, space6);
+
+// Check that the non-static resource One is initialized from binding on
+// startup (register 1, space 5).
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::__createFromBinding(unsigned int, unsigned int, int, unsigned int, char const*)
+// CHECK-SAME: (ptr {{.*}} @One, i32 noundef 1, i32 noundef 5, i32 noundef 1, i32 noundef 0, ptr noundef [[ONE_STR]])
+
+// Note that non-static resource arrays are not initialized on startup.
+// The individual resources from the array are initialized on access.
+
+static RWBuffer<float> StaticOne;
+static RWBuffer<float> StaticArray[2];
+
+// Check that StaticOne resource is initialized on startup with the default
+// constructor and not from binding. It will initalize the handle to poison.
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} @StaticOne)
+
+// Check that StaticArray elements are initialized on startup with the default
+// constructor and not from binding. The initializer will loop over the array
+// elements and call the default constructor for each one, setting the handle to poison.
+// CHECK: define internal void @__cxx_global_var_init{{.*}}
+// CHECK-NEXT: entry:
+// CHECK-NEXT: br label %arrayctor.loop
+// CHECK: arrayctor.loop: ; preds = %arrayctor.loop, %entry
+// CHECK-NEXT: %arrayctor.cur = phi ptr [ @StaticArray, %entry ], [ %arrayctor.next, %arrayctor.loop ]
+// CHECK-NEXT: call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} %arrayctor.cur)
+// CHECK-NEXT: %arrayctor.next = getelementptr inbounds %"class.hlsl::RWBuffer", ptr %arrayctor.cur, i32 1
+// CHECK-NEXT: %arrayctor.done = icmp eq ptr %arrayctor.next, getelementptr inbounds (%"class.hlsl::RWBuffer", ptr @StaticArray, i32 2)
+// CHECK-NEXT: br i1 %arrayctor.done, label %arrayctor.cont, label %arrayctor.loop
+// CHECK: arrayctor.cont: ; preds = %arrayctor.loop
+// CHECK-NEXT: ret void
+
+// No other global initialization routines should be present.
+// CHECK-NOT: define internal void @__cxx_global_var_init{{.*}}
+
+[numthreads(4,1,1)]
+void main() {
+// CHECK: define internal void @main()()
+// CHECK-NEXT: entry:
+// CHECK-NEXT: %[[TMP0:.*]] = alloca %"class.hlsl::RWBuffer"
+
+ static RWBuffer<float> StaticLocal;
+// Check that StaticLocal is initialized to by default constructor to poison and not from binding
+// call void @hlsl::RWBuffer<float>::RWBuffer()(ptr {{.*}} @main()::StaticLocal)
+
+ StaticLocal = Array[2][0];
+// A[2][0] is accessed here, so it should be initialized from binding (register 10, space 6, index 4),
+// and then assigned to StaticLocal using = operator.
+// CHECK: call void @hlsl::RWBuffer<float>::__createFromBinding(unsigned int, unsigned int, int, unsigned int, char const*)
+// CHECK-SAME: (ptr {{.*}} %[[TMP0]], i32 noundef 10, i32 noundef 6, i32 noundef 8, i32 noundef 4, ptr noundef [[ARRAY_STR]])
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=({{.*}})(ptr {{.*}} @main()::StaticLocal, ptr {{.*}} %[[TMP0]])
+
+ StaticOne = One;
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=({{.*}})(ptr {{.*}} @StaticOne, ptr {{.*}} @One)
+
+ StaticArray[1] = One;
+// CHECK-NEXT: call {{.*}} ptr @hlsl::RWBuffer<float>::operator=(hlsl::RWBuffer<float> const&)
+// CHECK-SAME: (ptr {{.*}} getelementptr inbounds ([2 x %"class.hlsl::RWBuffer"], ptr @StaticArray, i32 0, i32 1), ptr {{.*}} @One)
+
+ StaticLocal[0] = 123;
+// CHECK-NEXT: %[[PTR0:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)(ptr {{.*}} @main()::StaticLocal, i32 noundef 0)
+// CHECK-NEXT: store float 1.230000e+02, ptr %[[PTR0]]
+
+ StaticOne[1] = 456;
+// CHECK-NEXT: %[[PTR1:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)(ptr {{.*}}) @StaticOne, i32 noundef 1)
+// CHECK-NEXT: store float 4.560000e+02, ptr %[[PTR1]], align 4
+
+ StaticArray[1][2] = 789;
+// CHECK-NEXT: %[[PTR2:.*]] = call {{.*}} ptr @hlsl::RWBuffer<float>::operator[](unsigned int)
+// CHECK-SAME: (ptr {{.*}} getelementptr inbounds ([2 x %"class.hlsl::RWBuffer"], ptr @StaticArray, i32 0, i32 1), i32 noundef 2)
+// CHECK-NEXT: store float 7.890000e+02, ptr %[[PTR2]], align 4
+}
+
+// No other binding initialization calls should be present.
+// CHECK-NOT: call void @hlsl::RWBuffer<float>::__createFrom{{.*}}Binding{{.*}}
|
This change fixes couple of issues with static resources:
Depends on #166844