Skip to content

Commit 912c232

Browse files
s-perrons-perron
committed
Fixes from code review.
1 parent 6eb5c31 commit 912c232

File tree

4 files changed

+61
-50
lines changed

4 files changed

+61
-50
lines changed

‎clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.cpp‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -799,40 +799,32 @@ BuiltinTypeDeclBuilder::addMemberVariable(StringRef Name, QualType Type,
799799
return *this;
800800
}
801801

802-
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleMembers(
803-
ResourceClass RC, bool IsROV, bool RawBuffer, AccessSpecifier Access) {
802+
BuiltinTypeDeclBuilder &
803+
BuiltinTypeDeclBuilder::addBufferHandles(ResourceClass RC, bool IsROV,
804+
bool RawBuffer, bool HasCounter,
805+
AccessSpecifier Access) {
804806
addHandleMember(RC, IsROV, RawBuffer, Access);
805-
if (resourceHasCounter(Record)) {
807+
if (HasCounter) {
806808
addCounterHandleMember(RC, IsROV, RawBuffer, Access);
807809
}
808810
return *this;
809811
}
810812

811813
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addHandleMember(
812814
ResourceClass RC, bool IsROV, bool RawBuffer, AccessSpecifier Access) {
813-
assert(!Record->isCompleteDefinition() && "record is already complete");
814-
815-
ASTContext &Ctx = SemaRef.getASTContext();
816-
TypeSourceInfo *ElementTypeInfo =
817-
Ctx.getTrivialTypeSourceInfo(getHandleElementType(), SourceLocation());
818-
819-
// add handle member with resource type attributes
820-
QualType AttributedResTy = QualType();
821-
SmallVector<const Attr *> Attrs = {
822-
HLSLResourceClassAttr::CreateImplicit(Ctx, RC),
823-
IsROV ? HLSLROVAttr::CreateImplicit(Ctx) : nullptr,
824-
RawBuffer ? HLSLRawBufferAttr::CreateImplicit(Ctx) : nullptr,
825-
ElementTypeInfo
826-
? HLSLContainedTypeAttr::CreateImplicit(Ctx, ElementTypeInfo)
827-
: nullptr};
828-
if (CreateHLSLAttributedResourceType(SemaRef, Ctx.HLSLResourceTy, Attrs,
829-
AttributedResTy))
830-
addMemberVariable("__handle", AttributedResTy, {}, Access);
831-
return *this;
815+
return addResourceMember("__handle", RC, IsROV, RawBuffer,
816+
/*IsCounter=*/false, Access);
832817
}
833818

834819
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCounterHandleMember(
835820
ResourceClass RC, bool IsROV, bool RawBuffer, AccessSpecifier Access) {
821+
return addResourceMember("__counter_handle", RC, IsROV, RawBuffer,
822+
/*IsCounter=*/true, Access);
823+
}
824+
825+
BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addResourceMember(
826+
StringRef MemberName, ResourceClass RC, bool IsROV, bool RawBuffer,
827+
bool IsCounter, AccessSpecifier Access) {
836828
assert(!Record->isCompleteDefinition() && "record is already complete");
837829

838830
ASTContext &Ctx = SemaRef.getASTContext();
@@ -847,11 +839,13 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCounterHandleMember(
847839
RawBuffer ? HLSLRawBufferAttr::CreateImplicit(Ctx) : nullptr,
848840
ElementTypeInfo
849841
? HLSLContainedTypeAttr::CreateImplicit(Ctx, ElementTypeInfo)
850-
: nullptr,
851-
HLSLIsCounterAttr::CreateImplicit(Ctx)};
842+
: nullptr};
843+
if (IsCounter)
844+
Attrs.push_back(HLSLIsCounterAttr::CreateImplicit(Ctx));
845+
852846
if (CreateHLSLAttributedResourceType(SemaRef, Ctx.HLSLResourceTy, Attrs,
853847
AttributedResTy))
854-
addMemberVariable("__counter_handle", AttributedResTy, {}, Access);
848+
addMemberVariable(MemberName, AttributedResTy, {}, Access);
855849
return *this;
856850
}
857851

@@ -960,10 +954,9 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCopyConstructor() {
960954
.accessHandleFieldOnResource(PH::_0)
961955
.assign(PH::Handle, PH::LastStmt);
962956

963-
if (getResourceCounterHandleField()) {
957+
if (getResourceCounterHandleField())
964958
MMB.accessCounterHandleFieldOnResource(PH::_0).assign(PH::CounterHandle,
965959
PH::LastStmt);
966-
}
967960

968961
return MMB.finalize();
969962
}
@@ -984,10 +977,9 @@ BuiltinTypeDeclBuilder &BuiltinTypeDeclBuilder::addCopyAssignmentOperator() {
984977
.accessHandleFieldOnResource(PH::_0)
985978
.assign(PH::Handle, PH::LastStmt);
986979

987-
if (getResourceCounterHandleField()) {
980+
if (getResourceCounterHandleField())
988981
MMB.accessCounterHandleFieldOnResource(PH::_0).assign(PH::CounterHandle,
989982
PH::LastStmt);
990-
}
991983

992984
return MMB.returnThis().finalize();
993985
}
@@ -1026,7 +1018,8 @@ FieldDecl *BuiltinTypeDeclBuilder::getResourceHandleField() const {
10261018

10271019
FieldDecl *BuiltinTypeDeclBuilder::getResourceCounterHandleField() const {
10281020
auto I = Fields.find("__counter_handle");
1029-
if (I == Fields.end())
1021+
if (I == Fields.end() ||
1022+
!I->second->getType()->isHLSLAttributedResourceType())
10301023
return nullptr;
10311024
return I->second;
10321025
}

‎clang/lib/Sema/HLSLBuiltinTypeDeclBuilder.h‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,14 +72,9 @@ class BuiltinTypeDeclBuilder {
7272
AccessSpecifier Access = AccessSpecifier::AS_private);
7373

7474
BuiltinTypeDeclBuilder &
75-
addHandleMembers(ResourceClass RC, bool IsROV, bool RawBuffer,
75+
addBufferHandles(ResourceClass RC, bool IsROV, bool RawBuffer,
76+
bool HasCounter,
7677
AccessSpecifier Access = AccessSpecifier::AS_private);
77-
BuiltinTypeDeclBuilder &
78-
addHandleMember(ResourceClass RC, bool IsROV, bool RawBuffer,
79-
AccessSpecifier Access = AccessSpecifier::AS_private);
80-
BuiltinTypeDeclBuilder &
81-
addCounterHandleMember(ResourceClass RC, bool IsROV, bool RawBuffer,
82-
AccessSpecifier Access = AccessSpecifier::AS_private);
8378
BuiltinTypeDeclBuilder &addArraySubscriptOperators();
8479

8580
// Builtin types constructors
@@ -101,6 +96,16 @@ class BuiltinTypeDeclBuilder {
10196
BuiltinTypeDeclBuilder &addConsumeMethod();
10297

10398
private:
99+
BuiltinTypeDeclBuilder &addResourceMember(StringRef MemberName,
100+
ResourceClass RC, bool IsROV,
101+
bool RawBuffer, bool IsCounter,
102+
AccessSpecifier Access);
103+
BuiltinTypeDeclBuilder &
104+
addHandleMember(ResourceClass RC, bool IsROV, bool RawBuffer,
105+
AccessSpecifier Access = AccessSpecifier::AS_private);
106+
BuiltinTypeDeclBuilder &
107+
addCounterHandleMember(ResourceClass RC, bool IsROV, bool RawBuffer,
108+
AccessSpecifier Access = AccessSpecifier::AS_private);
104109
FieldDecl *getResourceHandleField() const;
105110
FieldDecl *getResourceCounterHandleField() const;
106111
QualType getFirstTemplateTypeParam();

‎clang/lib/Sema/HLSLExternalSemaSource.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,9 @@ void HLSLExternalSemaSource::defineTrivialHLSLTypes() {
230230
/// Set up common members and attributes for buffer types
231231
static BuiltinTypeDeclBuilder setupBufferType(CXXRecordDecl *Decl, Sema &S,
232232
ResourceClass RC, bool IsROV,
233-
bool RawBuffer) {
233+
bool RawBuffer, bool HasCounter) {
234234
return BuiltinTypeDeclBuilder(S, Decl)
235-
.addHandleMembers(RC, IsROV, RawBuffer)
235+
.addBufferHandles(RC, IsROV, RawBuffer, HasCounter)
236236
.addDefaultHandleConstructor()
237237
.addCopyConstructor()
238238
.addCopyAssignmentOperator()
@@ -377,7 +377,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
377377

378378
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
379379
setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, /*IsROV=*/false,
380-
/*RawBuffer=*/false)
380+
/*RawBuffer=*/false, /*HasCounter=*/false)
381381
.addArraySubscriptOperators()
382382
.addLoadMethods()
383383
.completeDefinition();
@@ -389,7 +389,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
389389

390390
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
391391
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/false,
392-
/*RawBuffer=*/false)
392+
/*RawBuffer=*/false, /*HasCounter=*/false)
393393
.addArraySubscriptOperators()
394394
.addLoadMethods()
395395
.completeDefinition();
@@ -401,7 +401,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
401401
.finalizeForwardDeclaration();
402402
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
403403
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/true,
404-
/*RawBuffer=*/false)
404+
/*RawBuffer=*/false, /*HasCounter=*/false)
405405
.addArraySubscriptOperators()
406406
.addLoadMethods()
407407
.completeDefinition();
@@ -412,7 +412,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
412412
.finalizeForwardDeclaration();
413413
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
414414
setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, /*IsROV=*/false,
415-
/*RawBuffer=*/true)
415+
/*RawBuffer=*/true, /*HasCounter=*/false)
416416
.addArraySubscriptOperators()
417417
.addLoadMethods()
418418
.completeDefinition();
@@ -423,7 +423,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
423423
.finalizeForwardDeclaration();
424424
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
425425
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/false,
426-
/*RawBuffer=*/true)
426+
/*RawBuffer=*/true, /*HasCounter=*/true)
427427
.addArraySubscriptOperators()
428428
.addLoadMethods()
429429
.addIncrementCounterMethod()
@@ -437,7 +437,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
437437
.finalizeForwardDeclaration();
438438
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
439439
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/false,
440-
/*RawBuffer=*/true)
440+
/*RawBuffer=*/true, /*HasCounter=*/true)
441441
.addAppendMethod()
442442
.completeDefinition();
443443
});
@@ -448,7 +448,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
448448
.finalizeForwardDeclaration();
449449
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
450450
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/false,
451-
/*RawBuffer=*/true)
451+
/*RawBuffer=*/true, /*HasCounter=*/true)
452452
.addConsumeMethod()
453453
.completeDefinition();
454454
});
@@ -459,7 +459,7 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
459459
.finalizeForwardDeclaration();
460460
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
461461
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/true,
462-
/*RawBuffer=*/true)
462+
/*RawBuffer=*/true, /*HasCounter=*/true)
463463
.addArraySubscriptOperators()
464464
.addLoadMethods()
465465
.addIncrementCounterMethod()
@@ -471,22 +471,22 @@ void HLSLExternalSemaSource::defineHLSLTypesWithForwardDeclarations() {
471471
.finalizeForwardDeclaration();
472472
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
473473
setupBufferType(Decl, *SemaPtr, ResourceClass::SRV, /*IsROV=*/false,
474-
/*RawBuffer=*/true)
474+
/*RawBuffer=*/true, /*HasCounter=*/false)
475475
.completeDefinition();
476476
});
477477
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace, "RWByteAddressBuffer")
478478
.finalizeForwardDeclaration();
479479
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
480480
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/false,
481-
/*RawBuffer=*/true)
481+
/*RawBuffer=*/true, /*HasCounter=*/false)
482482
.completeDefinition();
483483
});
484484
Decl = BuiltinTypeDeclBuilder(*SemaPtr, HLSLNamespace,
485485
"RasterizerOrderedByteAddressBuffer")
486486
.finalizeForwardDeclaration();
487487
onCompletion(Decl, [this](CXXRecordDecl *Decl) {
488488
setupBufferType(Decl, *SemaPtr, ResourceClass::UAV, /*IsROV=*/true,
489-
/*RawBuffer=*/true)
489+
/*RawBuffer=*/true, /*HasCounter=*/false)
490490
.completeDefinition();
491491
});
492492
}

‎clang/test/CodeGenHLSL/resources/RWStructuredBuffer-elementtype.hlsl‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,31 @@
22
// RUN: %clang_cc1 -triple spirv-unknown-vulkan1.3-compute -finclude-default-header -fnative-half-type -emit-llvm -o - %s | FileCheck %s -check-prefixes=SPV
33

44
// CHECK: %"class.hlsl::RWStructuredBuffer" = type { target("dx.RawBuffer", i16, 1, 0), target("dx.RawBuffer", i16, 1, 0) }
5+
// SPV: %"class.hlsl::RWStructuredBuffer" = type { target("spirv.VulkanBuffer", [0 x i16], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
56
// CHECK: %"class.hlsl::RWStructuredBuffer.0" = type { target("dx.RawBuffer", i16, 1, 0), target("dx.RawBuffer", i16, 1, 0) }
7+
// SPV: %"class.hlsl::RWStructuredBuffer.0" = type { target("spirv.VulkanBuffer", [0 x i16], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
68
// CHECK: %"class.hlsl::RWStructuredBuffer.1" = type { target("dx.RawBuffer", i32, 1, 0), target("dx.RawBuffer", i32, 1, 0) }
9+
// SPV: %"class.hlsl::RWStructuredBuffer.1" = type { target("spirv.VulkanBuffer", [0 x i32], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
710
// CHECK: %"class.hlsl::RWStructuredBuffer.2" = type { target("dx.RawBuffer", i32, 1, 0), target("dx.RawBuffer", i32, 1, 0) }
11+
// SPV: %"class.hlsl::RWStructuredBuffer.2" = type { target("spirv.VulkanBuffer", [0 x i32], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
812
// CHECK: %"class.hlsl::RWStructuredBuffer.3" = type { target("dx.RawBuffer", i64, 1, 0), target("dx.RawBuffer", i64, 1, 0) }
13+
// SPV: %"class.hlsl::RWStructuredBuffer.3" = type { target("spirv.VulkanBuffer", [0 x i64], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
914
// CHECK: %"class.hlsl::RWStructuredBuffer.4" = type { target("dx.RawBuffer", i64, 1, 0), target("dx.RawBuffer", i64, 1, 0) }
15+
// SPV: %"class.hlsl::RWStructuredBuffer.4" = type { target("spirv.VulkanBuffer", [0 x i64], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1016
// CHECK: %"class.hlsl::RWStructuredBuffer.5" = type { target("dx.RawBuffer", half, 1, 0), target("dx.RawBuffer", half, 1, 0) }
17+
// SPV: %"class.hlsl::RWStructuredBuffer.5" = type { target("spirv.VulkanBuffer", [0 x half], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1118
// CHECK: %"class.hlsl::RWStructuredBuffer.6" = type { target("dx.RawBuffer", float, 1, 0), target("dx.RawBuffer", float, 1, 0) }
19+
// SPV: %"class.hlsl::RWStructuredBuffer.6" = type { target("spirv.VulkanBuffer", [0 x float], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1220
// CHECK: %"class.hlsl::RWStructuredBuffer.7" = type { target("dx.RawBuffer", double, 1, 0), target("dx.RawBuffer", double, 1, 0) }
21+
// SPV: %"class.hlsl::RWStructuredBuffer.7" = type { target("spirv.VulkanBuffer", [0 x double], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1322
// CHECK: %"class.hlsl::RWStructuredBuffer.8" = type { target("dx.RawBuffer", <4 x i16>, 1, 0), target("dx.RawBuffer", <4 x i16>, 1, 0) }
23+
// SPV: %"class.hlsl::RWStructuredBuffer.8" = type { target("spirv.VulkanBuffer", [0 x <4 x i16>], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1424
// CHECK: %"class.hlsl::RWStructuredBuffer.9" = type { target("dx.RawBuffer", <3 x i32>, 1, 0), target("dx.RawBuffer", <3 x i32>, 1, 0) }
25+
// SPV: %"class.hlsl::RWStructuredBuffer.9" = type { target("spirv.VulkanBuffer", [0 x <3 x i32>], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1526
// CHECK: %"class.hlsl::RWStructuredBuffer.10" = type { target("dx.RawBuffer", <2 x half>, 1, 0), target("dx.RawBuffer", <2 x half>, 1, 0) }
27+
// SPV: %"class.hlsl::RWStructuredBuffer.10" = type { target("spirv.VulkanBuffer", [0 x <2 x half>], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1628
// CHECK: %"class.hlsl::RWStructuredBuffer.11" = type { target("dx.RawBuffer", <3 x float>, 1, 0), target("dx.RawBuffer", <3 x float>, 1, 0) }
29+
// SPV: %"class.hlsl::RWStructuredBuffer.11" = type { target("spirv.VulkanBuffer", [0 x <3 x float>], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1730
// CHECK: %"class.hlsl::RWStructuredBuffer.12" = type { target("dx.RawBuffer", i32, 1, 0), target("dx.RawBuffer", i32, 1, 0) }
1831
// SPV: %"class.hlsl::RWStructuredBuffer.12" = type { target("spirv.VulkanBuffer", [0 x i32], 12, 1), target("spirv.VulkanBuffer", i32, 12, 1) }
1932
// CHECK: %"class.hlsl::RWStructuredBuffer.13" = type { target("dx.RawBuffer", <4 x i32>, 1, 0), target("dx.RawBuffer", <4 x i32>, 1, 0) }

0 commit comments

Comments
 (0)