Skip to content

Commit 2eeb47e

Browse files
s-perrons-perron
authored
[SPIRV] Fix decoration attributes on members. (microsoft#7137)
This commit implements adding the `vk::ext_decorate` on a field. It also adds an error in sema if `vk::ext_decorate_id` or `vk::ext_decorate_string` are used on members. For `vk::ext_decorate_id`, there is no `OpMemberDecorateId` instruction, so we cannot add apply these decorations to members. For `vk::ext_decorate_string`, there is only one decoration that uses OpDecorateString or OpMemberDecorateString, UserSemantic. However, that decoration could have used OpDecorate or OpMemberDecorate because those instructions accept string literals. I do not expect any new decorations to use OpDecorateString or OpMemberDecorateString. I may eventually want to deprecate `vk::ext_decorate_string`. Fixes microsoft#4195
1 parent 92fcae2 commit 2eeb47e

File tree

9 files changed

+126
-38
lines changed

9 files changed

+126
-38
lines changed

‎tools/clang/include/clang/Basic/Attr.td

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,15 +1348,15 @@ def VKDecorateExt : InheritableAttr {
13481348

13491349
def VKDecorateIdExt : InheritableAttr {
13501350
let Spellings = [CXX11<"vk", "ext_decorate_id">];
1351-
let Subjects = SubjectList<[Function, Var, ParmVar, Field, TypedefName], ErrorDiag>;
1351+
let Subjects = SubjectList<[Function, Var, ParmVar, TypedefName], ErrorDiag>;
13521352
let Args = [UnsignedArgument<"decorate">, VariadicExprArgument<"arguments">];
13531353
let LangOpts = [SPIRV];
13541354
let Documentation = [Undocumented];
13551355
}
13561356

13571357
def VKDecorateStringExt : InheritableAttr {
13581358
let Spellings = [CXX11<"vk", "ext_decorate_string">];
1359-
let Subjects = SubjectList<[Function, Var, ParmVar, Field, TypedefName], ErrorDiag>;
1359+
let Subjects = SubjectList<[Function, Var, ParmVar, TypedefName], ErrorDiag>;
13601360
let Args = [UnsignedArgument<"decorate">, VariadicStringArgument<"arguments">];
13611361
let LangOpts = [SPIRV];
13621362
let Documentation = [Undocumented];

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

Lines changed: 36 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2333,36 +2333,42 @@ def err_duplicate_mangled_name : Error<
23332333
"definition with same mangled name as another definition">;
23342334
def err_cyclic_alias : Error<
23352335
"alias definition is part of a cycle">;
2336-
def warn_attribute_wrong_decl_type : Warning<
2337-
"%0 attribute only applies to %select{functions|unions|"
2338-
"variables and functions|functions and methods|parameters|"
2339-
"functions, methods and blocks|functions, methods, and classes|"
2340-
"functions, methods, and parameters|classes|enums|variables|methods|"
2341-
"variables, functions and labels|fields and global variables|structs|"
2342-
"variables and typedefs|thread-local variables|"
2343-
"variables and fields|variables, data members and tag types|"
2344-
"types and namespaces|Objective-C interfaces|methods and properties|"
2345-
"struct or union|struct, union or class|types|"
2346-
"Objective-C instance methods|init methods of interface or class extension declarations|"
2347-
"variables, functions and classes|Objective-C protocols|"
2348-
"functions and global variables|structs, unions, and typedefs|structs and typedefs|"
2349-
"interface or protocol declarations|kernel functions|"
2350-
// SPIRV Change Starts
2351-
"fields|"
2352-
"global variables of scalar type|"
2353-
"global variables of struct type|"
2354-
"global variables, cbuffers, and tbuffers|"
2355-
"Textures and Samplers|"
2356-
"RWTextures, RasterizerOrderedTextures, Buffers, RWBuffers, and RasterizerOrderedBuffers|"
2357-
"RWStructuredBuffers, AppendStructuredBuffers, and ConsumeStructuredBuffers|"
2358-
"SubpassInput, SubpassInputMS|"
2359-
"cbuffer or ConstantBuffer|"
2360-
// SPIRV Change Ends
2361-
// HLSL Change Starts - add 3 more enum values
2362-
"variables and parameters|functions, parameters, and fields|"
2363-
"functions, variables, parameters, fields, and types}1">,
2364-
// HLSL Change Ends
2365-
InGroup<IgnoredAttributes>;
2336+
def warn_attribute_wrong_decl_type
2337+
: Warning<
2338+
"%0 attribute only applies to %select{functions|unions|"
2339+
"variables and functions|functions and methods|parameters|"
2340+
"functions, methods and blocks|functions, methods, and classes|"
2341+
"functions, methods, and parameters|classes|enums|variables|methods|"
2342+
"variables, functions and labels|fields and global variables|structs|"
2343+
"variables and typedefs|thread-local variables|"
2344+
"variables and fields|variables, data members and tag types|"
2345+
"types and namespaces|Objective-C interfaces|methods and properties|"
2346+
"struct or union|struct, union or class|types|"
2347+
"Objective-C instance methods|init methods of interface or class "
2348+
"extension declarations|"
2349+
"variables, functions and classes|Objective-C protocols|"
2350+
"functions and global variables|structs, unions, and "
2351+
"typedefs|structs and typedefs|"
2352+
"interface or protocol declarations|kernel functions|"
2353+
// SPIRV Change Starts
2354+
"fields|"
2355+
"global variables of scalar type|"
2356+
"global variables of struct type|"
2357+
"global variables, cbuffers, and tbuffers|"
2358+
"Textures and Samplers|"
2359+
"RWTextures, RasterizerOrderedTextures, Buffers, RWBuffers, and "
2360+
"RasterizerOrderedBuffers|"
2361+
"RWStructuredBuffers, AppendStructuredBuffers, and "
2362+
"ConsumeStructuredBuffers|"
2363+
"SubpassInput, SubpassInputMS|"
2364+
"cbuffer or ConstantBuffer|"
2365+
"functions, variables, parameters, and types|"
2366+
// SPIRV Change Ends
2367+
// HLSL Change Starts - add 3 more enum values
2368+
"variables and parameters|functions, parameters, and fields|"
2369+
"functions, variables, parameters, fields, and types}1">,
2370+
// HLSL Change Ends
2371+
InGroup<IgnoredAttributes>;
23662372
def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Text>;
23672373
def warn_type_attribute_wrong_type : Warning<
23682374
"'%0' only applies to %select{function|pointer|"

‎tools/clang/include/clang/SPIRV/SpirvType.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,12 @@ class StructType : public SpirvType {
302302
llvm::Optional<uint32_t> offset_ = llvm::None,
303303
llvm::Optional<uint32_t> matrixStride_ = llvm::None,
304304
llvm::Optional<bool> isRowMajor_ = llvm::None,
305-
bool relaxedPrecision = false, bool precise = false)
305+
bool relaxedPrecision = false, bool precise = false,
306+
llvm::Optional<AttrVec> attributes = llvm::None)
306307
: type(type_), fieldIndex(fieldIndex_), name(name_), offset(offset_),
307308
sizeInBytes(llvm::None), matrixStride(matrixStride_),
308309
isRowMajor(isRowMajor_), isRelaxedPrecision(relaxedPrecision),
309-
isPrecise(precise) {
310+
isPrecise(precise), bitfield(llvm::None), attributes(attributes) {
310311
// A StructType may not contain any hybrid types.
311312
assert(!isa<HybridType>(type_));
312313
}
@@ -335,6 +336,8 @@ class StructType : public SpirvType {
335336
bool isPrecise;
336337
// Information about the bitfield (if applicable).
337338
llvm::Optional<BitfieldInfo> bitfield;
339+
// Other attributes applied to the field.
340+
llvm::Optional<AttrVec> attributes;
338341
};
339342

340343
StructType(
@@ -489,10 +492,11 @@ class HybridStructType : public HybridType {
489492
hlsl::ConstantPacking *packOffset = nullptr,
490493
const hlsl::RegisterAssignment *regC = nullptr,
491494
bool precise = false,
492-
llvm::Optional<BitfieldInfo> bitfield = llvm::None)
495+
llvm::Optional<BitfieldInfo> bitfield = llvm::None,
496+
llvm::Optional<AttrVec> attributes = llvm::None)
493497
: astType(astType_), name(name_), vkOffsetAttr(offset),
494498
packOffsetAttr(packOffset), registerC(regC), isPrecise(precise),
495-
bitfield(std::move(bitfield)) {}
499+
bitfield(std::move(bitfield)), attributes(std::move(attributes)) {}
496500

497501
// The field's type.
498502
QualType astType;
@@ -509,6 +513,8 @@ class HybridStructType : public HybridType {
509513
// Whether this field is a bitfield or not. If set to false, bitfield width
510514
// value is undefined.
511515
llvm::Optional<BitfieldInfo> bitfield;
516+
// Other attributes applied to the field.
517+
llvm::Optional<AttrVec> attributes;
512518
};
513519

514520
HybridStructType(

‎tools/clang/include/clang/Sema/AttributeList.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,7 @@ enum AttributeDeclKind {
867867
ExpectedCounterStructuredBuffer,
868868
ExpectedSubpassInput,
869869
ExpectedCTBuffer,
870+
ExpectedFunctionVariableParamOrTypedef,
870871
// SPIRV Change Ends
871872
// HLSL Change Begins - add attribute decl combinations
872873
ExpectedVariableOrParam,

‎tools/clang/lib/SPIRV/EmitVisitor.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2552,6 +2552,19 @@ uint32_t EmitTypeHandler::emitType(const SpirvType *type) {
25522552
// NonWritable decorations
25532553
if (structType->isReadOnly())
25542554
emitDecoration(id, spv::Decoration::NonWritable, {}, i);
2555+
2556+
if (field.attributes.hasValue()) {
2557+
for (auto &attr : field.attributes.getValue()) {
2558+
if (auto decorateExtAttr = dyn_cast<VKDecorateExtAttr>(attr)) {
2559+
emitDecoration(
2560+
id,
2561+
static_cast<spv::Decoration>(decorateExtAttr->getDecorate()),
2562+
{decorateExtAttr->literals_begin(),
2563+
decorateExtAttr->literals_end()},
2564+
i);
2565+
}
2566+
}
2567+
}
25552568
}
25562569

25572570
// Emit Block or BufferBlock decorations if necessary.

‎tools/clang/lib/SPIRV/LowerTypeVisitor.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,15 +1105,19 @@ LowerTypeVisitor::lowerStructFields(const RecordDecl *decl,
11051105
field->getBitWidthValue(field->getASTContext());
11061106
}
11071107

1108+
llvm::Optional<AttrVec> attributes;
11081109
if (field->hasAttrs()) {
1109-
for (auto &attr : field->getAttrs()) {
1110+
attributes.emplace();
1111+
for (auto attr : field->getAttrs()) {
11101112
if (auto capAttr = dyn_cast<VKCapabilityExtAttr>(attr)) {
11111113
spvBuilder.requireCapability(
11121114
static_cast<spv::Capability>(capAttr->getCapability()),
11131115
capAttr->getLocation());
11141116
} else if (auto extAttr = dyn_cast<VKExtensionExtAttr>(attr)) {
11151117
spvBuilder.requireExtension(extAttr->getName(),
11161118
extAttr->getLocation());
1119+
} else {
1120+
attributes->push_back(attr);
11171121
}
11181122
}
11191123
}
@@ -1124,7 +1128,8 @@ LowerTypeVisitor::lowerStructFields(const RecordDecl *decl,
11241128
/*packoffset*/ getPackOffset(field),
11251129
/*RegisterAssignment*/ nullptr,
11261130
/*isPrecise*/ field->hasAttr<HLSLPreciseAttr>(),
1127-
/*bitfield*/ bitfieldInfo));
1131+
/*bitfield*/ bitfieldInfo,
1132+
/* attributes */ attributes));
11281133
}
11291134

11301135
return populateLayoutInformation(fields, rule);
@@ -1210,6 +1215,7 @@ LowerTypeVisitor::lowerField(const HybridStructType::FieldInfo *field,
12101215
loweredField.isPrecise = true;
12111216
}
12121217
loweredField.bitfield = field->bitfield;
1218+
loweredField.attributes = field->attributes;
12131219

12141220
// We only need layout information for structures with non-void layout rule.
12151221
if (rule == SpirvLayoutRule::Void) {

‎tools/clang/test/CodeGenSPIRV/inline-spirv/spv.inline.decorate.member.hlsl

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %dxc -T ps_6_0 -E main -Vd -spirv %s -spirv | FileCheck %s
2+
3+
template<class T, class U>
4+
[[vk::ext_instruction(/*spv::OpBitcast*/124)]]
5+
T Bitcast(U);
6+
7+
// CHECK: OpMemberDecorate %S 0 Offset 0
8+
// CHECK: OpMemberDecorate %S 1 Offset 16
9+
// CHECK: %S = OpTypeStruct %v4float %v4float
10+
11+
struct S
12+
{
13+
[[vk::ext_decorate(/*offset*/ 35, 0)]] float4 f1;
14+
[[vk::ext_decorate(/*offset*/ 35, 16)]] float4 f2;
15+
};
16+
17+
using PointerType = vk::SpirvOpaqueType<
18+
/* OpTypePointer */ 32,
19+
/* PhysicalStorageBuffer */ vk::Literal<vk::integral_constant<uint,5349> >,
20+
S>;
21+
22+
[[vk::ext_capability(/*PhysicalStorageBufferAddresses */ 5347 )]]
23+
[[vk::ext_instruction( /*OpLoad*/ 61 )]]
24+
S Load(PointerType pointer,
25+
[[vk::ext_literal]] uint32_t __aligned=/*Aligned*/0x00000002,
26+
[[vk::ext_literal]] uint32_t __alignment=32);
27+
28+
uint64_t address;
29+
30+
float4 main() : SV_TARGET
31+
{
32+
33+
// CHECK: [[BC:%[0-9]+]] = OpBitcast %_ptr_PhysicalStorageBuffer_S {{%[0-9]+}}
34+
PointerType ptr = Bitcast<PointerType>(address);
35+
36+
// CHECK: [[LD:%[0-9]+]] = OpLoad %S [[BC]] Aligned 32
37+
// CHECK: [[RET:%[0-9]+]] = OpCompositeExtract %v4float [[LD]] 0
38+
// CHECK: OpStore %out_var_SV_TARGET [[RET]]
39+
return Load(ptr).f1;
40+
}

‎tools/clang/test/SemaHLSL/attributes/spv.inline.decorate.member.hlsl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// RUN: %dxc -T ps_6_0 -E main -verify -spirv %s
2+
3+
struct S
4+
{
5+
[[vk::ext_decorate_id(/*offset*/ 35, 0)]] float4 f1; /* expected-error{{'ext_decorate_id' attribute only applies to functions, variables, parameters, and types}} */
6+
[[vk::ext_decorate_string(/*offset*/ 35, "16")]] float4 f2; /* expected-error{{'ext_decorate_string' attribute only applies to functions, variables, parameters, and types}} */
7+
};
8+
9+
float4 main() : SV_TARGET
10+
{
11+
12+
}

‎tools/clang/utils/TableGen/ClangAttrEmitter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,10 @@ static std::string CalculateDiagnostic(const Record &S) {
23272327
case ObjCProtocol | ObjCInterface:
23282328
return "ExpectedObjectiveCInterfaceOrProtocol";
23292329
case Field | Var: return "ExpectedFieldOrGlobalVar";
2330+
// SPIRV Changes Start
2331+
case Func | Var | Param | Type:
2332+
return "ExpectedFunctionVariableParamOrTypedef";
2333+
// SPIRV Changes End
23302334
// HLSL Changes Start
23312335
case Var | Param:
23322336
return "ExpectedVariableOrParam";

0 commit comments

Comments
 (0)