Skip to content

Commit

Permalink
[SPIR-V] Allow built-in object types as arguments to SpirvType
Browse files Browse the repository at this point in the history
There has been a validation error for passing in a built-in type to a
template since the first commit to DXC. This check exists to prevent
instances like this:

```
Texture2D<SamplerState> image;
```

When DXC was first created, users could not create their own templates,
and therefore having a generic check for this did not cause any
problems. However, now that HLSL 2021 allows users to create their own
templates, and inline SPIR-V lets users pass types in to the SpirvType
and SpirvOpaqueType templates in order to reference them, this is
causing problems.

This PR allows the SpirvType and SpirvOpaqueType templates to take
HLSL built-in types as arguments.

Fixes microsoft#6498
  • Loading branch information
cassiebeckley committed Oct 8, 2024
1 parent b48341e commit 7d2702d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
21 changes: 14 additions & 7 deletions tools/clang/lib/Sema/SemaHLSL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4904,7 +4904,7 @@ class HLSLExternalSource : public ExternalSemaSource {
}

bool IsValidTemplateArgumentType(SourceLocation argLoc, const QualType &type,
bool requireScalar) {
bool requireScalar, bool allowObject) {
if (type.isNull()) {
return false;
}
Expand All @@ -4929,7 +4929,7 @@ class HLSLExternalSource : public ExternalSemaSource {
if (qt->isArrayType()) {
const ArrayType *arrayType = qt->getAsArrayTypeUnsafe();
return IsValidTemplateArgumentType(argLoc, arrayType->getElementType(),
false);
false, allowObject);
} else if (objectKind == AR_TOBJ_VECTOR) {
bool valid = true;
if (!IsValidVectorSize(GetHLSLVecSize(type))) {
Expand Down Expand Up @@ -4964,9 +4964,12 @@ class HLSLExternalSource : public ExternalSemaSource {
objectKind = ClassifyRecordType(recordType);
switch (objectKind) {
case AR_TOBJ_OBJECT:
m_sema->Diag(argLoc, diag::err_hlsl_objectintemplateargument) << type;
return false;
case AR_TOBJ_COMPOUND: {
if (objectKind == AR_TOBJ_OBJECT && !allowObject) {
m_sema->Diag(argLoc, diag::err_hlsl_objectintemplateargument)
<< type;
return false;
}
const RecordDecl *recordDecl = recordType->getDecl();
if (recordDecl->isInvalidDecl())
return false;
Expand All @@ -4975,8 +4978,9 @@ class HLSLExternalSource : public ExternalSemaSource {
bool result = true;
while (begin != end) {
const FieldDecl *fieldDecl = *begin;
if (!IsValidTemplateArgumentType(argLoc, fieldDecl->getType(),
false)) {
if (!IsValidTemplateArgumentType(
argLoc, fieldDecl->getType(), false,
allowObject && objectKind != AR_TOBJ_OBJECT)) {
m_sema->Diag(argLoc, diag::note_field_type_usage)
<< fieldDecl->getType() << fieldDecl->getIdentifier() << type;
result = false;
Expand Down Expand Up @@ -5193,7 +5197,10 @@ class HLSLExternalSource : public ExternalSemaSource {
QualType argType = arg.getAsType();
// Skip dependent types. Types will be checked later, when concrete.
if (!argType->isDependentType()) {
if (!IsValidTemplateArgumentType(argSrcLoc, argType, requireScalar)) {
bool allowObject =
templateName == "SpirvType" || templateName == "SpirvOpaqueType";
if (!IsValidTemplateArgumentType(argSrcLoc, argType, requireScalar,
allowObject)) {
// NOTE: IsValidTemplateArgumentType emits its own diagnostics
return true;
}
Expand Down
11 changes: 3 additions & 8 deletions tools/clang/test/CodeGenSPIRV/spv.inline.type.hlsl
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
// RUN: %dxc -T ps_6_0 -E main -fcgl %s -spirv | FileCheck %s

// TODO(6498): enable Array test when using `Texture2D` with an alias template of `SpirvType` is fixed
// CHECK-TODO: %type_Array_type_2d_image = OpTypeArray %type_2d_image
// template<typename SomeType>
// using Array = vk::SpirvOpaqueType</* OpTypeArray */ 28, SomeType, 4>;

// CHECK: %spirvIntrinsicType = OpTypeArray %type_2d_image %uint_4
typedef vk::SpirvOpaqueType</* OpTypeArray */ 28, Texture2D, vk::integral_constant<uint, 4> > ArrayTex2D;
template<typename SomeType>
using Array = vk::SpirvOpaqueType</* OpTypeArray */ 28, SomeType, vk::integral_constant<uint, 4> >;

// CHECK: %spirvIntrinsicType_0 = OpTypeInt 8 0
using uint8_t [[vk::ext_capability(/* Int8 */ 39)]] = vk::SpirvType</* OpTypeInt */ 21, 8, 8, vk::Literal<vk::integral_constant<uint, 8> >, vk::Literal<vk::integral_constant<bool, false> > >;
Expand All @@ -15,8 +11,7 @@ using uint8_t [[vk::ext_capability(/* Int8 */ 39)]] = vk::SpirvType</* OpTypeInt

void main() {
// CHECK: %image = OpVariable %_ptr_Function_spirvIntrinsicType Function
// Array<Texture2D> image;
ArrayTex2D image;
Array<Texture2D> image;

// CHECK: %byte = OpVariable %_ptr_Function_spirvIntrinsicType_0
uint8_t byte;
Expand Down

0 comments on commit 7d2702d

Please sign in to comment.