Skip to content

Commit 3b2afe7

Browse files
bob80905bob80905
committed
address all of Justin and Xiang's feedback
1 parent f985ab6 commit 3b2afe7

File tree

8 files changed

+51
-30
lines changed

8 files changed

+51
-30
lines changed

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12154,7 +12154,8 @@ def err_hlsl_missing_semantic_annotation : Error<
1215412154
def err_hlsl_init_priority_unsupported : Error<
1215512155
"initializer priorities are not supported in HLSL">;
1215612156

12157-
def err_hlsl_mismatching_register_type_and_name: Error<"invalid register name prefix '%0' for register type '%1' (expected %select{'t'|'u'|'b'|'s'}2)">;
12157+
def err_hlsl_mismatching_register_resource_type_and_name: Error<"invalid register name prefix '%0' for register resource type '%1' (expected %select{'t'|'u'|'b'|'t'|'s'}2)">;
12158+
def err_hlsl_mismatching_register_builtin_type_and_name: Error<"invalid register name prefix '%0' for '%1' (expected %2)">;
1215812159
def err_hlsl_unsupported_register_type : Error<"invalid resource class specifier '%0' used; expected 'b', 's', 't', or 'u'">;
1215912160
def err_hlsl_unsupported_register_number : Error<"register number should be an integer">;
1216012161
def err_hlsl_expected_space : Error<"invalid space specifier '%0' used; expected 'space' followed by an integer, like space1">;

‎clang/lib/Sema/HLSLExternalSemaSource.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,8 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
477477
}
478478

479479
/// Set up common members and attributes for buffer types
480-
static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
481-
ResourceClass RC) {
480+
static BuiltinTypeDeclBuilder setupBufferHandle(CXXRecordDecl *Decl, Sema &S,
481+
ResourceClass RC) {
482482
return BuiltinTypeDeclBuilder(Decl)
483483
.addHandleMember()
484484
.addDefaultHandleConstructor(S, RC);
@@ -494,7 +494,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
494494
.Record;
495495

496496
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
497-
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
497+
setupBufferHandle(Decl, *SemaPtr, ResourceClass::UAV)
498498
.addArraySubscriptOperators()
499499
.completeDefinition();
500500
});
@@ -506,7 +506,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
506506
/*IsROV=*/true)
507507
.Record;
508508
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
509-
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV)
509+
setupBufferHandle(Decl, *SemaPtr, ResourceClass::UAV)
510510
.addArraySubscriptOperators()
511511
.completeDefinition();
512512
});

‎clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7344,11 +7344,23 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
73447344
return;
73457345

73467346
llvm::hlsl::ResourceClass DeclResourceClass;
7347-
StringRef varTy = "";
7347+
StringRef VarTy = "";
73487348
if (SamplerUAVOrSRV) {
73497349
const Type *Ty = SamplerUAVOrSRV->getType()->getPointeeOrArrayElementType();
73507350
if (!Ty)
7351-
llvm_unreachable("Resource class should have an element type.");
7351+
llvm_unreachable("Resource class must have an element type.");
7352+
7353+
if (const BuiltinType *BTy = dyn_cast<BuiltinType>(Ty)) {
7354+
QualType QT = SamplerUAVOrSRV->getType();
7355+
PrintingPolicy PP = S.getPrintingPolicy();
7356+
std::string typestr = QualType::getAsString(QT.split(), PP);
7357+
7358+
if (Slot[0] != 'b' && Slot[0] != 'c' && Slot[0] != 'i')
7359+
S.Diag(ArgLoc,
7360+
diag::err_hlsl_mismatching_register_builtin_type_and_name)
7361+
<< Slot.substr(0, 1) << typestr << "'b, c, or i";
7362+
return;
7363+
}
73527364

73537365
const CXXRecordDecl *TheRecordDecl = Ty->getAsCXXRecordDecl();
73547366
if (!TheRecordDecl)
@@ -7363,42 +7375,49 @@ static void DiagnoseHLSLResourceRegType(Sema &S, SourceLocation &ArgLoc,
73637375
llvm_unreachable("Resource class should have a resource attribute.");
73647376

73657377
DeclResourceClass = Attr->getResourceClass();
7366-
varTy = TheRecordDecl->getName();
7378+
VarTy = TheRecordDecl->getName();
73677379
} else {
73687380
if (CBufferOrTBuffer->isCBuffer()) {
73697381
DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
7370-
varTy = "cbuffer";
7382+
VarTy = "cbuffer";
73717383
} else {
7372-
DeclResourceClass = llvm::hlsl::ResourceClass::CBuffer;
7373-
varTy = "tbuffer";
7384+
DeclResourceClass = llvm::hlsl::ResourceClass::TBuffer;
7385+
VarTy = "tbuffer";
73747386
}
73757387
}
73767388
switch (DeclResourceClass) {
73777389
case llvm::hlsl::ResourceClass::SRV: {
73787390
if (Slot[0] != 't')
7379-
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
7380-
<< Slot.substr(0, 1) << varTy
7391+
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
7392+
<< Slot.substr(0, 1) << VarTy
73817393
<< (unsigned)llvm::hlsl::ResourceClass::SRV;
73827394
break;
73837395
}
73847396
case llvm::hlsl::ResourceClass::UAV: {
73857397
if (Slot[0] != 'u')
7386-
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
7387-
<< Slot.substr(0, 1) << varTy
7398+
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
7399+
<< Slot.substr(0, 1) << VarTy
73887400
<< (unsigned)llvm::hlsl::ResourceClass::UAV;
73897401
break;
73907402
}
73917403
case llvm::hlsl::ResourceClass::CBuffer: {
73927404
if (Slot[0] != 'b')
7393-
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
7394-
<< Slot.substr(0, 1) << varTy
7405+
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
7406+
<< Slot.substr(0, 1) << VarTy
73957407
<< (unsigned)llvm::hlsl::ResourceClass::CBuffer;
73967408
break;
73977409
}
7410+
case llvm::hlsl::ResourceClass::TBuffer: {
7411+
if (Slot[0] != 't')
7412+
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
7413+
<< Slot.substr(0, 1) << VarTy
7414+
<< (unsigned)llvm::hlsl::ResourceClass::TBuffer;
7415+
break;
7416+
}
73987417
case llvm::hlsl::ResourceClass::Sampler: {
73997418
if (Slot[0] != 's')
7400-
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_type_and_name)
7401-
<< Slot.substr(0, 1) << varTy
7419+
S.Diag(ArgLoc, diag::err_hlsl_mismatching_register_resource_type_and_name)
7420+
<< Slot.substr(0, 1) << VarTy
74027421
<< (unsigned)llvm::hlsl::ResourceClass::Sampler;
74037422
break;
74047423
}

‎clang/test/AST/HLSL/resource_binding_attr.hlsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ cbuffer CB : register(b3, space2) {
88
}
99

1010
// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:13:9 tbuffer TB
11-
// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "b2" "space1"
11+
// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}} <col:14> "t2" "space1"
1212
// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
13-
tbuffer TB : register(b2, space1) {
13+
tbuffer TB : register(t2, space1) {
1414
float b;
1515
}
1616

‎clang/test/CodeGenHLSL/cbuf.hlsl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ cbuffer A : register(b0, space2) {
99
}
1010

1111
// CHECK: @[[TB:.+]] = external constant { float, double }
12-
tbuffer A : register(b2, space1) {
12+
tbuffer A : register(t2, space1) {
1313
float c;
1414
double d;
1515
}

‎clang/test/SemaHLSL/resource_binding_attr_error.hlsl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ void foo2() {
4444
// expected-warning@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
4545
extern RWBuffer<float> U2 : register(u5);
4646
}
47-
// FIXME: expect-error once fix https://github.com/llvm/llvm-project/issues/57886.
48-
// float b : register(u0, space1) {}
47+
// expected-error@+1 {{invalid register name prefix 'u' for 'float' (expected 'b, c, or i)}}
48+
float b : register(u0, space1);
4949

5050
// expected-warning@+1 {{'register' attribute only applies to cbuffer/tbuffer and external global variables}}
5151
void bar(RWBuffer<float> U : register(u3)) {

‎clang/test/SemaHLSL/resource_binding_attr_error_mismatch.hlsl

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
22

33
// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
4-
// NOT YET IMPLEMENTED : {{invalid register name prefix 'b' for register type 'RWBuffer' (expected 'u')}}
5-
// NOT YET IMPLEMENTED RWBuffer<int> a : register(b2, space1);
4+
// expected-error@+1 {{invalid register name prefix 'b' for register resource type 'RWBuffer' (expected 'u')}}
5+
RWBuffer<int> a : register(b2, space1);
66

77
// the below will cause an llvm unreachable, because RWBuffers don't have resource attributes yet
8-
// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'RWBuffer' (expected 'u')}}
9-
// NOT YET IMPLEMENTED RWBuffer<int> b : register(t2, space1);
8+
// expected-error@+1 {{invalid register name prefix 't' for register resource type 'RWBuffer' (expected 'u')}}
9+
RWBuffer<int> b : register(t2, space1);
1010

1111
// NOT YET IMPLEMENTED : {{invalid register name prefix 'u' for register type 'Texture1D' (expected 't')}}
1212
// NOT YET IMPLEMENTED Texture1D<float> tex : register(u3);
@@ -47,14 +47,14 @@
4747
// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'StructuredBuffer' (expected 'u')}}
4848
// NOT YET IMPLEMENTED StructuredBuffer ROVStructuredBuff_t2 : register(T2);
4949

50-
// expected-error@+1 {{invalid register name prefix 's' for register type 'cbuffer' (expected 'b')}}
50+
// expected-error@+1 {{invalid register name prefix 's' for register resource type 'cbuffer' (expected 'b')}}
5151
cbuffer f : register(s2, space1) {}
5252

5353
// NOT YET IMPLEMENTED : {{invalid register name prefix 't' for register type 'Sampler' (expected 's')}}
5454
// Can this type just be Sampler instead of SamplerState?
5555
// NOT YET IMPLEMENTED SamplerState MySampler : register(t3, space1);
5656

57-
// expected-error@+1 {{invalid register name prefix 's' for register type 'tbuffer' (expected 'b')}}
57+
// expected-error@+1 {{invalid register name prefix 's' for register resource type 'tbuffer' (expected 't')}}
5858
tbuffer f : register(s2, space1) {}
5959

6060
// NOT YET IMPLEMENTED : RTAccelerationStructure doesn't have any example tests in DXC

‎llvm/include/llvm/Frontend/HLSL/HLSLResource.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ enum class ResourceClass : uint8_t {
2525
SRV = 0,
2626
UAV,
2727
CBuffer,
28+
TBuffer,
2829
Sampler,
2930
Invalid,
3031
NumClasses = Invalid,

0 commit comments

Comments
 (0)