From ffe3753c8d767c7c88a889ea0ba5bbf403ea56e4 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 12 Apr 2018 08:58:01 -0400 Subject: [PATCH 01/15] [spirv] Stop checking descriptor binding number overlap (#1210) The Vulkan 1.1.72 spec says: > It is valid for multiple shader variables to be assigned the same > descriptor set and binding values, as long as all those that are > statically used by the entry point being compiled are compatible > with the descriptor type in the descriptor set layout binding. --- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 91 ++++++------------- tools/clang/lib/SPIRV/DeclResultIdMapper.h | 19 +--- .../CodeGenSPIRV/vk.binding.cl.error.hlsl | 30 ------ .../vk.binding.explicit.error.hlsl | 42 --------- .../vk.binding.register.error.hlsl | 29 ------ .../unittests/SPIRV/CodeGenSPIRVTest.cpp | 9 -- 6 files changed, 28 insertions(+), 192 deletions(-) delete mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.cl.error.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.explicit.error.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.register.error.hlsl diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index a4c43c532..98cc43e92 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -37,15 +37,6 @@ const hlsl::RegisterAssignment *getResourceBinding(const NamedDecl *decl) { return nullptr; } -/// \brief Returns the resource category for the given type. -ResourceVar::Category getResourceCategory(QualType type) { - if (TypeTranslator::isTexture(type) || TypeTranslator::isRWTexture(type)) - return ResourceVar::Category::Image; - if (TypeTranslator::isSampler(type)) - return ResourceVar::Category::Sampler; - return ResourceVar::Category::Other; -} - /// \brief Returns true if the given declaration has a primitive type qualifier. /// Returns false otherwise. inline bool hasGSPrimitiveTypeQualifier(const Decl *decl) { @@ -436,8 +427,7 @@ SpirvEvalInfo DeclResultIdMapper::createExternVar(const VarDecl *var) { const auto *bindingAttr = var->getAttr(); const auto *counterBindingAttr = var->getAttr(); - resourceVars.emplace_back(id, getResourceCategory(var->getType()), regAttr, - bindingAttr, counterBindingAttr); + resourceVars.emplace_back(id, regAttr, bindingAttr, counterBindingAttr); if (const auto *inputAttachment = var->getAttr()) theBuilder.decorateInputAttachmentIndex(id, inputAttachment->getIndex()); @@ -577,9 +567,9 @@ uint32_t DeclResultIdMapper::createCTBuffer(const HLSLBufferDecl *decl) { : spirvOptions.tBufferLayoutRule); astDecls[varDecl].indexInCTBuffer = index++; } - resourceVars.emplace_back( - bufferVar, ResourceVar::Category::Other, getResourceBinding(decl), - decl->getAttr(), decl->getAttr()); + resourceVars.emplace_back(bufferVar, getResourceBinding(decl), + decl->getAttr(), + decl->getAttr()); return bufferVar; } @@ -615,9 +605,9 @@ uint32_t DeclResultIdMapper::createCTBuffer(const VarDecl *decl) { .setStorageClass(spv::StorageClass::Uniform) .setLayoutRule(context->isCBuffer() ? spirvOptions.cBufferLayoutRule : spirvOptions.tBufferLayoutRule); - resourceVars.emplace_back( - bufferVar, ResourceVar::Category::Other, getResourceBinding(context), - decl->getAttr(), decl->getAttr()); + resourceVars.emplace_back(bufferVar, getResourceBinding(context), + decl->getAttr(), + decl->getAttr()); return bufferVar; } @@ -652,8 +642,7 @@ void DeclResultIdMapper::createGlobalsCBuffer(const VarDecl *var) { context, /*arraySize*/ 0, ContextUsageKind::Globals, "type.$Globals", "$Globals"); - resourceVars.emplace_back(globals, ResourceVar::Category::Other, nullptr, - nullptr, nullptr); + resourceVars.emplace_back(globals, nullptr, nullptr, nullptr); uint32_t index = 0; for (const auto *decl : typeTranslator.collectDeclsInDeclContext(context)) @@ -760,8 +749,7 @@ void DeclResultIdMapper::createCounterVar( if (!isAlias) { // Non-alias counter variables should be put in to resourceVars so that // descriptors can be allocated for them. - resourceVars.emplace_back(counterId, ResourceVar::Category::Other, - getResourceBinding(decl), + resourceVars.emplace_back(counterId, getResourceBinding(decl), decl->getAttr(), decl->getAttr(), true); assert(declId); @@ -861,39 +849,24 @@ private: /// set and binding number. class BindingSet { public: - /// Tries to use the given set and binding number. Returns true if possible, - /// false otherwise and writes the source location of where the binding number - /// is used to *usedLoc. - bool tryToUseBinding(uint32_t binding, uint32_t set, - ResourceVar::Category category, SourceLocation tryLoc, - SourceLocation *usedLoc) { - const auto cat = static_cast(category); - // Note that we will create the entry for binding in bindings[set] here. - // But that should not have bad effects since it defaults to zero. - if ((usedBindings[set][binding] & cat) == 0) { - usedBindings[set][binding] |= cat; - whereUsed[set][binding] = tryLoc; - return true; - } - *usedLoc = whereUsed[set][binding]; - return false; + /// Uses the given set and binding number. + void useBinding(uint32_t binding, uint32_t set) { + usedBindings[set].insert(binding); } /// Uses the next avaiable binding number in set 0. - uint32_t useNextBinding(uint32_t set, ResourceVar::Category category) { + uint32_t useNextBinding(uint32_t set) { auto &binding = usedBindings[set]; auto &next = nextBindings[set]; while (binding.count(next)) ++next; - binding[next] = static_cast(category); + binding.insert(next); return next++; } private: - ///< set number -> (binding number -> resource category) - llvm::DenseMap> usedBindings; - ///< set number -> (binding number -> source location) - llvm::DenseMap> whereUsed; + ///< set number -> set of used binding number + llvm::DenseMap> usedBindings; ///< set number -> next available binding number llvm::DenseMap nextBindings; }; @@ -1086,19 +1059,11 @@ bool DeclResultIdMapper::decorateResourceBindings() { BindingSet bindingSet; // Decorates the given varId of the given category with set number - // setNo, binding number bindingNo. Emits warning if overlap. - const auto tryToDecorate = [this, &bindingSet]( - const uint32_t varId, const uint32_t setNo, - const uint32_t bindingNo, - const ResourceVar::Category cat, - SourceLocation loc) { - SourceLocation prevUseLoc; - if (!bindingSet.tryToUseBinding(bindingNo, setNo, cat, loc, &prevUseLoc)) { - emitWarning("resource binding #%0 in descriptor set #%1 already assigned", - loc) - << bindingNo << setNo; - emitNote("binding number previously assigned here", prevUseLoc); - } + // setNo, binding number bindingNo. Ignores overlaps. + const auto tryToDecorate = [this, &bindingSet](const uint32_t varId, + const uint32_t setNo, + const uint32_t bindingNo) { + bindingSet.useBinding(bindingNo, setNo); theBuilder.decorateDSetBinding(varId, setNo, bindingNo); }; @@ -1112,15 +1077,13 @@ bool DeclResultIdMapper::decorateResourceBindings() { if (const auto *reg = var.getRegister()) set = reg->RegisterSpace; - tryToDecorate(var.getSpirvId(), set, vkCBinding->getBinding(), - var.getCategory(), vkCBinding->getLocation()); + tryToDecorate(var.getSpirvId(), set, vkCBinding->getBinding()); } } else { if (const auto *vkBinding = var.getBinding()) { // Process m1 tryToDecorate(var.getSpirvId(), vkBinding->getSet(), - vkBinding->getBinding(), var.getCategory(), - vkBinding->getLocation()); + vkBinding->getBinding()); } } } @@ -1156,12 +1119,10 @@ bool DeclResultIdMapper::decorateResourceBindings() { llvm_unreachable("unknown register type found"); } - tryToDecorate(var.getSpirvId(), set, binding, var.getCategory(), - reg->Loc); + tryToDecorate(var.getSpirvId(), set, binding); } for (const auto &var : resourceVars) { - const auto cat = var.getCategory(); if (var.isCounter()) { if (!var.getCounterBinding()) { // Process mX * c2 @@ -1172,12 +1133,12 @@ bool DeclResultIdMapper::decorateResourceBindings() { set = reg->RegisterSpace; theBuilder.decorateDSetBinding(var.getSpirvId(), set, - bindingSet.useNextBinding(set, cat)); + bindingSet.useNextBinding(set)); } } else if (!var.getBinding() && !var.getRegister()) { // Process m3 theBuilder.decorateDSetBinding(var.getSpirvId(), 0, - bindingSet.useNextBinding(0, cat)); + bindingSet.useNextBinding(0)); } } diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.h b/tools/clang/lib/SPIRV/DeclResultIdMapper.h index 6a345e4af..733ad5767 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.h +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.h @@ -103,27 +103,13 @@ private: class ResourceVar { public: - /// The category of this resource. - /// - /// We only care about Vulkan image and sampler types here, since they can be - /// bundled together as a combined image sampler which takes the same binding - /// number. The compiler should allow this case. - /// - /// Numbers are assigned to make bit check easiser. - enum class Category : uint32_t { - Image = 1, - Sampler = 2, - Other = 3, - }; - - ResourceVar(uint32_t id, Category cat, const hlsl::RegisterAssignment *r, + ResourceVar(uint32_t id, const hlsl::RegisterAssignment *r, const VKBindingAttr *b, const VKCounterBindingAttr *cb, bool counter = false) - : varId(id), category(cat), reg(r), binding(b), counterBinding(cb), + : varId(id), reg(r), binding(b), counterBinding(cb), isCounterVar(counter) {} uint32_t getSpirvId() const { return varId; } - Category getCategory() const { return category; } const hlsl::RegisterAssignment *getRegister() const { return reg; } const VKBindingAttr *getBinding() const { return binding; } bool isCounter() const { return isCounterVar; } @@ -131,7 +117,6 @@ public: private: uint32_t varId; ///< - Category category; ///< Resource category const hlsl::RegisterAssignment *reg; ///< HLSL register assignment const VKBindingAttr *binding; ///< Vulkan binding assignment const VKCounterBindingAttr *counterBinding; ///< Vulkan counter binding diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.error.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.error.hlsl deleted file mode 100644 index fb34b2021..000000000 --- a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.error.hlsl +++ /dev/null @@ -1,30 +0,0 @@ -// Run: %dxc -T ps_6_0 -E main -fvk-b-shift 2 0 -fvk-t-shift 2 0 -fvk-s-shift 3 0 -fvk-u-shift 3 0 - -struct S { - float4 f; -}; - -[[vk::binding(2)]] -ConstantBuffer cbuffer3; - -ConstantBuffer cbuffer1 : register(b0); // Collision with cbuffer3 after shift - -Texture2D texture1: register(t0, space1); -Texture2D texture2: register(t0); // Collision with cbuffer3 after shift - -SamplerState sampler1: register(s0); -SamplerState sampler2: register(s0, space2); - -RWBuffer rwbuffer1 : register(u0, space3); -RWBuffer rwbuffer2 : register(u0); // Collision with sampler1 after shift - -float4 main() : SV_Target { - return cbuffer1.f; -} - -//CHECK: :10:30: warning: resource binding #2 in descriptor set #0 already assigned -//CHECK: :7:3: note: binding number previously assigned here - -//CHECK: :13:29: warning: resource binding #2 in descriptor set #0 already assigned - -//CHECK: :19:30: warning: resource binding #3 in descriptor set #0 already assigned \ No newline at end of file diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.explicit.error.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.explicit.error.hlsl deleted file mode 100644 index a2509aa0f..000000000 --- a/tools/clang/test/CodeGenSPIRV/vk.binding.explicit.error.hlsl +++ /dev/null @@ -1,42 +0,0 @@ -// Run: %dxc -T ps_6_0 -E main - -[[vk::binding(1)]] -SamplerState sampler1 : register(s1, space1); - -[[vk::binding(3, 1)]] -SamplerState sampler2 : register(s2); - -[[vk::binding(1)]] // reuse - allowed for combined image sampler -Texture2D texture1; - -[[vk::binding(3, 1)]] // reuse - allowed for combined image sampler -Texture3D texture2 : register(t0, space0); - -[[vk::binding(3, 1)]] // reuse - disallowed -Texture3D texture3 : register(t0, space0); - -[[vk::binding(1)]] // reuse - disallowed -SamplerState sampler3 : register(s1, space1); - -struct S { float f; }; - -[[vk::binding(5)]] -StructuredBuffer buf1; - -[[vk::binding(5)]] // reuse - disallowed -SamplerState sampler4; - -[[vk::binding(5)]] // reuse - disallowed -Texture2D texture4; - -float4 main() : SV_Target { - return 1.0; -} - -// CHECK-NOT: :9:{{%\d+}}: warning: resource binding #1 in descriptor set #0 already assigned -// CHECK-NOT: :12:{{%\d+}}: warning: resource binding #3 in descriptor set #1 already assigned -// CHECK: :15:3: warning: resource binding #3 in descriptor set #1 already assigned -// CHECK: :12:3: note: binding number previously assigned here -// CHECK: :18:3: warning: resource binding #1 in descriptor set #0 already assigned -// CHECK: :26:3: warning: resource binding #5 in descriptor set #0 already assigned -// CHECK: :29:3: warning: resource binding #5 in descriptor set #0 already assigned diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.register.error.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.register.error.hlsl deleted file mode 100644 index dd278f6c4..000000000 --- a/tools/clang/test/CodeGenSPIRV/vk.binding.register.error.hlsl +++ /dev/null @@ -1,29 +0,0 @@ -// Run: %dxc -T ps_6_0 -E main - -struct S { - float4 f; -}; - -ConstantBuffer myCbuffer1 : register(b0); -ConstantBuffer myCbuffer2 : register(b0, space1); - -RWStructuredBuffer mySBuffer1 : register(u0); // reuse - disallowed -RWStructuredBuffer mySBuffer2 : register(u0, space1); // reuse - disallowed -RWStructuredBuffer mySBuffer3 : register(u0, space2); - -SamplerState mySampler1 : register(s5, space1); -Texture2D myTexture1 : register(t5, space1); // reuse - allowed - -Texture2D myTexture2 : register(t6, space6); -[[vk::binding(6, 6)]] // reuse - allowed -SamplerState mySampler2; - -float4 main() : SV_Target { - return 1.0; -} - -// CHECK: :10:36: warning: resource binding #0 in descriptor set #0 already assigned -// CHECK: :7:36: note: binding number previously assigned here -// CHECK: :11:36: warning: resource binding #0 in descriptor set #1 already assigned -// CHECK-NOT: :15:{{%\d+}}: warning: resource binding #5 in descriptor set #1 already assigned -// CHECK-NOT: :18:{{%\d+}}: warning: resource binding #6 in descriptor set #6 already assigned diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index 3d227b987..7e28da5ce 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -1321,15 +1321,6 @@ TEST_F(FileTest, VulkanRegisterBindingShift) { // command line option runFileTest("vk.binding.cl.hlsl"); } -TEST_F(FileTest, VulkanExplicitBindingReassigned) { - runFileTest("vk.binding.explicit.error.hlsl", Expect::Warning); -} -TEST_F(FileTest, VulkanRegisterBindingReassigned) { - runFileTest("vk.binding.register.error.hlsl", Expect::Warning); -} -TEST_F(FileTest, VulkanRegisterBindingShiftReassigned) { - runFileTest("vk.binding.cl.error.hlsl", Expect::Warning); -} TEST_F(FileTest, VulkanStructuredBufferCounter) { // [[vk::counter_binding()]] for RWStructuredBuffer, AppendStructuredBuffer, // and ConsumeStructuredBuffer From 6d152bf14c9a183d4c814068e292604515d0bb86 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Thu, 12 Apr 2018 10:06:55 -0400 Subject: [PATCH 02/15] [spirv] Boolean layout in struct reconstruction. (#1211) --- tools/clang/lib/SPIRV/SPIRVEmitter.cpp | 44 ++++++++++++++- tools/clang/lib/SPIRV/TypeTranslator.cpp | 4 +- tools/clang/lib/SPIRV/TypeTranslator.h | 2 +- .../vk.layout.cbuffer.boolean.hlsl | 42 +++++++++++++- .../vk.layout.rwstructuredbuffer.boolean.hlsl | 56 +++++++++++++++++++ 5 files changed, 141 insertions(+), 7 deletions(-) diff --git a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp index e5ba2a493..751b04c1e 100644 --- a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp +++ b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp @@ -4824,6 +4824,45 @@ void SPIRVEmitter::storeValue(const SpirvEvalInfo &lhsPtr, uint32_t SPIRVEmitter::reconstructValue(const SpirvEvalInfo &srcVal, const QualType valType, LayoutRule dstLR) { + // Lambda for casting scalar or vector of bool<-->uint in cases where one side + // of the reconstruction (lhs or rhs) has a layout rule. + const auto handleBooleanLayout = [this, &srcVal, dstLR](uint32_t val, + QualType valType) { + // We only need to cast if we have a scalar or vector of booleans. + if (!isBoolOrVecOfBoolType(valType)) + return val; + + LayoutRule srcLR = srcVal.getLayoutRule(); + // Source value has a layout rule, and has therefore been represented + // as a uint. Cast it to boolean before using. + bool shouldCastToBool = + srcLR != LayoutRule::Void && dstLR == LayoutRule::Void; + // Destination has a layout rule, and should therefore be represented + // as a uint. Cast to uint before using. + bool shouldCastToUint = + srcLR == LayoutRule::Void && dstLR != LayoutRule::Void; + // No boolean layout issues to take care of. + if (!shouldCastToBool && !shouldCastToUint) + return val; + + uint32_t vecSize = 1; + TypeTranslator::isVectorType(valType, nullptr, &vecSize); + QualType boolType = + vecSize == 1 ? astContext.BoolTy + : astContext.getExtVectorType(astContext.BoolTy, vecSize); + QualType uintType = + vecSize == 1 + ? astContext.UnsignedIntTy + : astContext.getExtVectorType(astContext.UnsignedIntTy, vecSize); + + if (shouldCastToBool) + return castToBool(val, uintType, boolType); + if (shouldCastToUint) + return castToInt(val, boolType, uintType, {}); + + return val; + }; + // Lambda for cases where we want to reconstruct an array const auto reconstructArray = [this, &srcVal, valType, dstLR](uint32_t arraySize, @@ -4834,7 +4873,6 @@ uint32_t SPIRVEmitter::reconstructValue(const SpirvEvalInfo &srcVal, typeTranslator.translateType(arrayElemType, srcVal.getLayoutRule()); const auto subSrcVal = theBuilder.createCompositeExtract(subSrcValType, srcVal, {i}); - elements.push_back(reconstructValue(srcVal.substResultId(subSrcVal), arrayElemType, dstLR)); } @@ -4868,7 +4906,7 @@ uint32_t SPIRVEmitter::reconstructValue(const SpirvEvalInfo &srcVal, // Note: This check should happen before the RecordType check since // vector/matrix/resource types are represented as RecordType in the AST. if (hlsl::IsHLSLVecMatType(valType) || hlsl::IsHLSLResourceType(valType)) - return srcVal; + return handleBooleanLayout(srcVal, valType); // Structs if (const auto *recordType = valType->getAs()) { @@ -4888,7 +4926,7 @@ uint32_t SPIRVEmitter::reconstructValue(const SpirvEvalInfo &srcVal, return theBuilder.createCompositeConstruct(dstValType, elements); } - return srcVal; + return handleBooleanLayout(srcVal, valType); } SpirvEvalInfo SPIRVEmitter::processBinaryOp(const Expr *lhs, const Expr *rhs, diff --git a/tools/clang/lib/SPIRV/TypeTranslator.cpp b/tools/clang/lib/SPIRV/TypeTranslator.cpp index 6260120a3..609aad39a 100644 --- a/tools/clang/lib/SPIRV/TypeTranslator.cpp +++ b/tools/clang/lib/SPIRV/TypeTranslator.cpp @@ -1281,10 +1281,10 @@ void TypeTranslator::collectDeclsInField( return; } - (*decls).push_back(field); + decls->push_back(field); } -const llvm::SmallVector +llvm::SmallVector TypeTranslator::collectDeclsInDeclContext(const DeclContext *declContext) { llvm::SmallVector decls; for (const auto *field : declContext->decls()) { diff --git a/tools/clang/lib/SPIRV/TypeTranslator.h b/tools/clang/lib/SPIRV/TypeTranslator.h index e31fd86b6..9f81a1069 100644 --- a/tools/clang/lib/SPIRV/TypeTranslator.h +++ b/tools/clang/lib/SPIRV/TypeTranslator.h @@ -274,7 +274,7 @@ public: /// DeclContext. If it sees a NamespaceDecl, it recursively dives in and /// collects decls in the correct order. /// Utilizes collectDeclsInNamespace and collectDeclsInField private methods. - const llvm::SmallVector + llvm::SmallVector collectDeclsInDeclContext(const DeclContext *declContext); private: diff --git a/tools/clang/test/CodeGenSPIRV/vk.layout.cbuffer.boolean.hlsl b/tools/clang/test/CodeGenSPIRV/vk.layout.cbuffer.boolean.hlsl index cfd977a85..3c245de16 100644 --- a/tools/clang/test/CodeGenSPIRV/vk.layout.cbuffer.boolean.hlsl +++ b/tools/clang/test/CodeGenSPIRV/vk.layout.cbuffer.boolean.hlsl @@ -7,12 +7,18 @@ // CHECK: OpMemberDecorate %type_CONSTANTS 0 Offset 0 // CHECK: OpDecorate %type_CONSTANTS Block -// CHECK: %FrameConstants = OpTypeStruct %uint %v3uint %_arr_v3uint_uint_2 +// CHECK: %T = OpTypeStruct %_arr_uint_uint_1 +struct T { + bool boolArray[1]; +}; + +// CHECK: %FrameConstants = OpTypeStruct %uint %v3uint %_arr_v3uint_uint_2 %T struct FrameConstants { bool boolScalar; bool3 boolVec; row_major bool2x3 boolMat; + T t; }; [[vk::binding(0, 0)]] @@ -23,6 +29,11 @@ cbuffer CONSTANTS // CHECK: [[v3uint0:%\d+]] = OpConstantComposite %v3uint %uint_0 %uint_0 %uint_0 // CHECK: [[v2uint0:%\d+]] = OpConstantComposite %v2uint %uint_0 %uint_0 + +// These are the types that hold SPIR-V booleans, rather than Uints. +// CHECK: %T_0 = OpTypeStruct %_arr_bool_uint_1 +// CHECK: %FrameConstants_0 = OpTypeStruct %bool %v3bool %_arr_v3bool_uint_2 %T_0 + float4 main(in float4 texcoords : TEXCOORD0) : SV_TARGET { // CHECK: [[FrameConstants:%\d+]] = OpAccessChain %_ptr_Uniform_FrameConstants %CONSTANTS %int_0 @@ -118,5 +129,34 @@ float4 main(in float4 texcoords : TEXCOORD0) : SV_TARGET // CHECK-NEXT: OpStore %k [[boolVec]] bool2 k = frameConstants.boolMat._m12_m01; +// CHECK: [[FrameConstants:%\d+]] = OpAccessChain %_ptr_Uniform_FrameConstants %CONSTANTS %int_0 +// CHECK-NEXT: [[ptr:%\d+]] = OpAccessChain %_ptr_Uniform_uint [[FrameConstants]] %int_3 %int_0 %int_2 +// CHECK-NEXT: [[uint:%\d+]] = OpLoad %uint [[ptr]] +// CHECK-NEXT: [[bool:%\d+]] = OpINotEqual %bool [[uint]] %uint_0 +// CHECK-NEXT: OpStore %l [[bool]] + bool l = frameConstants.t.boolArray[2]; + +// CHECK: [[FrameConstantsPtr:%\d+]] = OpAccessChain %_ptr_Uniform_FrameConstants %CONSTANTS %int_0 +// CHECK-NEXT: [[FrameConstants:%\d+]] = OpLoad %FrameConstants [[FrameConstantsPtr]] +// CHECK-NEXT: [[fc_0_uint:%\d+]] = OpCompositeExtract %uint [[FrameConstants]] 0 +// CHECK-NEXT: [[fc_0_bool:%\d+]] = OpINotEqual %bool [[fc_0_uint]] %uint_0 +// CHECK-NEXT: [[fc_1_uint3:%\d+]] = OpCompositeExtract %v3uint [[FrameConstants]] 1 +// CHECK-NEXT: [[fc_1_bool3:%\d+]] = OpINotEqual %v3bool [[fc_1_uint3]] [[v3uint0]] +// CHECK-NEXT: [[fc_2_uintMat:%\d+]] = OpCompositeExtract %_arr_v3uint_uint_2 [[FrameConstants]] 2 +// CHECK-NEXT: [[fc_2_uintMat_row0_uint:%\d+]] = OpCompositeExtract %v3uint [[fc_2_uintMat]] 0 +// CHECK-NEXT: [[fc_2_uintMat_row0_bool:%\d+]] = OpINotEqual %v3bool [[fc_2_uintMat_row0_uint]] [[v3uint0]] +// CHECK-NEXT: [[fc_2_uintMat_row1_uint:%\d+]] = OpCompositeExtract %v3uint [[fc_2_uintMat]] 1 +// CHECK-NEXT: [[fc_2_uintMat_row1_bool:%\d+]] = OpINotEqual %v3bool [[fc_2_uintMat_row1_uint]] [[v3uint0]] +// CHECK-NEXT: [[fc_2_boolMat:%\d+]] = OpCompositeConstruct %_arr_v3bool_uint_2 [[fc_2_uintMat_row0_bool]] [[fc_2_uintMat_row1_bool]] +// CHECK-NEXT: [[fc_3_T:%\d+]] = OpCompositeExtract %T [[FrameConstants]] 3 +// CHECK-NEXT: [[fc_3_T_0_uint_arr:%\d+]] = OpCompositeExtract %_arr_uint_uint_1 [[fc_3_T]] 0 +// CHECK-NEXT: [[fc_3_T_0_0_uint:%\d+]] = OpCompositeExtract %uint [[fc_3_T_0_uint_arr]] 0 +// CHECK-NEXT: [[fc_3_T_0_0_bool:%\d+]] = OpINotEqual %bool [[fc_3_T_0_0_uint]] %uint_0 +// CHECK-NEXT: [[fc_3_T_0_bool_arr:%\d+]] = OpCompositeConstruct %_arr_bool_uint_1 [[fc_3_T_0_0_bool]] +// CHECK-NEXT: [[fc_3_T_bool:%\d+]] = OpCompositeConstruct %T_0 [[fc_3_T_0_bool_arr]] +// CHECK-NEXT: [[fc:%\d+]] = OpCompositeConstruct %FrameConstants_0 [[fc_0_bool]] [[fc_1_bool3]] [[fc_2_boolMat]] [[fc_3_T_bool]] +// CHECK-NEXT: OpStore %fc [[fc]] + FrameConstants fc = frameConstants; + return (1.0).xxxx; } diff --git a/tools/clang/test/CodeGenSPIRV/vk.layout.rwstructuredbuffer.boolean.hlsl b/tools/clang/test/CodeGenSPIRV/vk.layout.rwstructuredbuffer.boolean.hlsl index 94e12d597..3a808b1ba 100644 --- a/tools/clang/test/CodeGenSPIRV/vk.layout.rwstructuredbuffer.boolean.hlsl +++ b/tools/clang/test/CodeGenSPIRV/vk.layout.rwstructuredbuffer.boolean.hlsl @@ -1,16 +1,28 @@ // Run: %dxc -T vs_6_0 -E main +// CHECK: %T = OpTypeStruct %_arr_uint_uint_1 +struct T { + bool boolArray[1]; +}; + +// CHECK: %S = OpTypeStruct %uint %v3uint %_arr_v3uint_uint_2 %T struct S { bool boolScalar; bool3 boolVec; row_major bool2x3 boolMat; + T t; }; RWStructuredBuffer values; // CHECK: [[v3uint1:%\d+]] = OpConstantComposite %v3uint %uint_1 %uint_1 %uint_1 // CHECK: [[v3uint0:%\d+]] = OpConstantComposite %v3uint %uint_0 %uint_0 %uint_0 + +// These are the types that hold SPIR-V booleans, rather than Uints. +// CHECK: %T_0 = OpTypeStruct %_arr_bool_uint_1 +// CHECK: %S_0 = OpTypeStruct %bool %v3bool %_arr_v3bool_uint_2 %T_0 + void main() { bool3 boolVecVar; @@ -32,4 +44,48 @@ void main() // values[2].boolMat = boolMatVar; // values[0].boolVec.yzx = boolVecVar; // values[0].boolMat._m12_m11 = boolVecVar2; + +// CHECK: [[sPtr:%\d+]] = OpAccessChain %_ptr_Uniform_S %values %int_0 %uint_0 +// CHECK-NEXT: [[s:%\d+]] = OpLoad %S [[sPtr]] +// CHECK-NEXT: [[s0:%\d+]] = OpCompositeExtract %uint [[s]] 0 +// CHECK-NEXT: [[s0_bool:%\d+]] = OpINotEqual %bool [[s0]] %uint_0 +// CHECK-NEXT: [[s1:%\d+]] = OpCompositeExtract %v3uint [[s]] 1 +// CHECK-NEXT: [[s1_bool:%\d+]] = OpINotEqual %v3bool [[s1]] [[v3uint0]] +// CHECK-NEXT: [[s2:%\d+]] = OpCompositeExtract %_arr_v3uint_uint_2 [[s]] 2 +// CHECK-NEXT: [[s2_row0:%\d+]] = OpCompositeExtract %v3uint [[s2]] 0 +// CHECK-NEXT: [[s2_row0_bool:%\d+]] = OpINotEqual %v3bool [[s2_row0]] [[v3uint0]] +// CHECK-NEXT: [[s2_row1:%\d+]] = OpCompositeExtract %v3uint [[s2]] 1 +// CHECK-NEXT: [[s2_row1_bool:%\d+]] = OpINotEqual %v3bool [[s2_row1]] [[v3uint0]] +// CHECK-NEXT: [[s2_bool:%\d+]] = OpCompositeConstruct %_arr_v3bool_uint_2 [[s2_row0_bool]] [[s2_row1_bool]] +// CHECK-NEXT: [[t:%\d+]] = OpCompositeExtract %T [[s]] 3 +// CHECK-NEXT: [[t0_uint_arr:%\d+]] = OpCompositeExtract %_arr_uint_uint_1 [[t]] 0 +// CHECK-NEXT: [[t0_0_uint:%\d+]] = OpCompositeExtract %uint [[t0_uint_arr]] 0 +// CHECK-NEXT: [[t0_0_bool:%\d+]] = OpINotEqual %bool [[t0_0_uint]] %uint_0 +// CHECK-NEXT: [[t0_bool:%\d+]] = OpCompositeConstruct %_arr_bool_uint_1 [[t0_0_bool]] +// CHECK-NEXT: [[t_bool:%\d+]] = OpCompositeConstruct %T_0 [[t0_bool]] +// CHECK-NEXT: [[s_bool:%\d+]] = OpCompositeConstruct %S_0 [[s0_bool]] [[s1_bool]] [[s2_bool]] [[t_bool]] +// CHECK-NEXT: OpStore %s [[s_bool]] + S s = values[0]; + +// CHECK: [[s:%\d+]] = OpLoad %S_0 %s +// CHECK-NEXT: [[resultPtr:%\d+]] = OpAccessChain %_ptr_Uniform_S %values %int_0 %uint_1 +// CHECK-NEXT: [[s0_bool:%\d+]] = OpCompositeExtract %bool [[s]] 0 +// CHECK-NEXT: [[s0_uint:%\d+]] = OpSelect %uint [[s0_bool]] %uint_1 %uint_0 +// CHECK-NEXT: [[s1_boolVec:%\d+]] = OpCompositeExtract %v3bool [[s]] 1 +// CHECK-NEXT: [[s1_uintVec:%\d+]] = OpSelect %v3uint [[s1_boolVec]] +// CHECK-NEXT: [[s2_boolMat:%\d+]] = OpCompositeExtract %_arr_v3bool_uint_2 [[s]] 2 +// CHECK-NEXT: [[s2_boolMat_row0:%\d+]] = OpCompositeExtract %v3bool [[s2_boolMat]] 0 +// CHECK-NEXT: [[s2_boolMat_row0_uint:%\d+]] = OpSelect %v3uint [[s2_boolMat_row0]] +// CHECK-NEXT: [[s2_boolMat_row1:%\d+]] = OpCompositeExtract %v3bool [[s2_boolMat]] 1 +// CHECK-NEXT: [[s2_boolMat_row1_uint:%\d+]] = OpSelect %v3uint [[s2_boolMat_row1]] +// CHECK-NEXT: [[s2_uintMat:%\d+]] = OpCompositeConstruct %_arr_v3uint_uint_2 [[s2_boolMat_row0_uint]] [[s2_boolMat_row1_uint]] +// CHECK-NEXT: [[t:%\d+]] = OpCompositeExtract %T_0 [[s]] 3 +// CHECK-NEXT: [[t0_bool_arr:%\d+]] = OpCompositeExtract %_arr_bool_uint_1 [[t]] 0 +// CHECK-NEXT: [[t0_bool_arr_0:%\d+]] = OpCompositeExtract %bool [[t0_bool_arr]] 0 +// CHECK-NEXT: [[t0_bool_arr_0_uint:%\d+]] = OpSelect %uint [[t0_bool_arr_0]] %uint_1 %uint_0 +// CHECK-NEXT: [[t0_uint_arr:%\d+]] = OpCompositeConstruct %_arr_uint_uint_1 [[t0_bool_arr_0_uint]] +// CHECK-NEXT: [[t_uint:%\d+]] = OpCompositeConstruct %T [[t0_uint_arr]] +// CHECK-NEXT: [[s_uint:%\d+]] = OpCompositeConstruct %S [[s0_uint]] [[s1_uintVec]] [[s2_uintMat]] [[t_uint]] +// CHECK-NEXT: OpStore [[resultPtr:%\d+]] [[s_uint]] + values[1] = s; } From 794840785fdfe70f3d8b7701e2ea77b0f6d5069a Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 12 Apr 2018 10:09:16 -0400 Subject: [PATCH 03/15] [spirv] Support signed integer types for some builtins (#1215) * SV_DispatchThreadID * SV_GroupID * SV_GroupThreadID --- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 23 +++++++++++-------- .../semantic.dispatch-thread-id.int2.cs.hlsl | 13 +++++++++++ .../semantic.dispatch-thread-id.uint2.cs.hlsl | 12 ---------- .../semantic.group-id.int2.cs.hlsl | 13 +++++++++++ .../semantic.group-id.uint2.cs.hlsl | 12 ---------- .../semantic.group-thread-id.int2.cs.hlsl | 13 +++++++++++ .../semantic.group-thread-id.uint2.cs.hlsl | 12 ---------- .../unittests/SPIRV/CodeGenSPIRVTest.cpp | 12 +++++----- 8 files changed, 58 insertions(+), 52 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.int2.cs.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.uint2.cs.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/semantic.group-id.int2.cs.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/semantic.group-id.uint2.cs.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.int2.cs.hlsl delete mode 100644 tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.uint2.cs.hlsl diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 98cc43e92..dd156bb23 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1243,11 +1243,9 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint, // SampleMask, must be an array of integers. // * SV_InnerCoverage is an uint value, but the corresponding builtin, // FullyCoveredEXT, must be an boolean value. - // * SV_DispatchThreadID and SV_GroupThreadID are allowed to be uint, uint2, - // or uint3, but the corresponding builtins (GlobalInvocationId and - // LocalInvocationId) must be a uint3. - // * SV_GroupID is allowed to be uint, uint2, or uint3, but the - // corresponding builtin (WorkgroupId) must be a uint3. + // * SV_DispatchThreadID, SV_GroupThreadID, and SV_GroupID are allowed to be + // uint, uint2, or uint3, but the corresponding builtins + // (GlobalInvocationId, LocalInvocationId, WorkgroupId) must be a uint3. if (glPerVertex.tryToAccess(sigPoint->GetKind(), semanticKind, semanticToUse->index, invocationId, value, @@ -1255,6 +1253,7 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint, return true; const uint32_t srcTypeId = typeId; // Variable type in source code + uint32_t srcVecElemTypeId = 0; // Variable element type if vector switch (semanticKind) { case hlsl::Semantic::Kind::DomainLocation: @@ -1280,7 +1279,10 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint, case hlsl::Semantic::Kind::DispatchThreadID: case hlsl::Semantic::Kind::GroupThreadID: case hlsl::Semantic::Kind::GroupID: - typeId = theBuilder.getVecType(theBuilder.getUint32Type(), 3); + // Keep the original integer signedness + srcVecElemTypeId = typeTranslator.translateType( + hlsl::IsHLSLVecType(type) ? hlsl::GetHLSLVecElementType(type) : type); + typeId = theBuilder.getVecType(srcVecElemTypeId, 3); break; } @@ -1419,15 +1421,16 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint, semanticKind == hlsl::Semantic::Kind::GroupID) && (!hlsl::IsHLSLVecType(type) || hlsl::GetHLSLVecSize(type) != 3)) { + assert(srcVecElemTypeId); const auto vecSize = hlsl::IsHLSLVecType(type) ? hlsl::GetHLSLVecSize(type) : 1; if (vecSize == 1) - *value = theBuilder.createCompositeExtract(theBuilder.getUint32Type(), - *value, {0}); + *value = + theBuilder.createCompositeExtract(srcVecElemTypeId, *value, {0}); else if (vecSize == 2) *value = theBuilder.createVectorShuffle( - theBuilder.getVecType(theBuilder.getUint32Type(), 2), *value, - *value, {0, 1}); + theBuilder.getVecType(srcVecElemTypeId, 2), *value, *value, + {0, 1}); } } else { if (noWriteBack) diff --git a/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.int2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.int2.cs.hlsl new file mode 100644 index 000000000..80a2efeac --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.int2.cs.hlsl @@ -0,0 +1,13 @@ +// Run: %dxc -T cs_6_0 -E main + +// CHECK: OpEntryPoint GLCompute %main "main" %gl_GlobalInvocationID +// CHECK: OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId +// CHECK: %gl_GlobalInvocationID = OpVariable %_ptr_Input_v3int Input + +// CHECK: %param_var_tid = OpVariable %_ptr_Function_v2int Function +// CHECK: [[gl_GlobalInvocationID:%\d+]] = OpLoad %v3int %gl_GlobalInvocationID +// CHECK: [[int2_DispatchThreadID:%\d+]] = OpVectorShuffle %v2int [[gl_GlobalInvocationID]] [[gl_GlobalInvocationID]] 0 1 +// CHECK: OpStore %param_var_tid [[int2_DispatchThreadID]] + +[numthreads(8, 8, 8)] +void main(int2 tid : SV_DispatchThreadId) {} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.uint2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.uint2.cs.hlsl deleted file mode 100644 index 1e5ac0b76..000000000 --- a/tools/clang/test/CodeGenSPIRV/semantic.dispatch-thread-id.uint2.cs.hlsl +++ /dev/null @@ -1,12 +0,0 @@ -// Run: %dxc -T cs_6_0 -E main - -// CHECK: OpEntryPoint GLCompute %main "main" %gl_GlobalInvocationID -// CHECK: OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId -// CHECK: %gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input - -// CHECK: [[gl_GlobalInvocationID:%\d+]] = OpLoad %v3uint %gl_GlobalInvocationID -// CHECK: [[uint2_DispatchThreadID:%\d+]] = OpVectorShuffle %v2uint [[gl_GlobalInvocationID]] [[gl_GlobalInvocationID]] 0 1 -// CHECK: OpStore %param_var_tid [[uint2_DispatchThreadID]] - -[numthreads(8, 8, 8)] -void main(uint2 tid : SV_DispatchThreadId) {} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.group-id.int2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.group-id.int2.cs.hlsl new file mode 100644 index 000000000..d926812a0 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/semantic.group-id.int2.cs.hlsl @@ -0,0 +1,13 @@ +// Run: %dxc -T cs_6_0 -E main + +// CHECK: OpEntryPoint GLCompute %main "main" %gl_WorkGroupID +// CHECK: OpDecorate %gl_WorkGroupID BuiltIn WorkgroupId +// CHECK: %gl_WorkGroupID = OpVariable %_ptr_Input_v3int Input + +// CHECK: %param_var_tid = OpVariable %_ptr_Function_v2int Function +// CHECK: [[gl_WorkGrouID:%\d+]] = OpLoad %v3int %gl_WorkGroupID +// CHECK: [[int2_GroupID:%\d+]] = OpVectorShuffle %v2int [[gl_WorkGrouID]] [[gl_WorkGrouID]] 0 1 +// CHECK: OpStore %param_var_tid [[int2_GroupID]] + +[numthreads(8, 8, 8)] +void main(int2 tid : SV_GroupID) {} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.group-id.uint2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.group-id.uint2.cs.hlsl deleted file mode 100644 index 55586b433..000000000 --- a/tools/clang/test/CodeGenSPIRV/semantic.group-id.uint2.cs.hlsl +++ /dev/null @@ -1,12 +0,0 @@ -// Run: %dxc -T cs_6_0 -E main - -// CHECK: OpEntryPoint GLCompute %main "main" %gl_WorkGroupID -// CHECK: OpDecorate %gl_WorkGroupID BuiltIn WorkgroupId -// CHECK: %gl_WorkGroupID = OpVariable %_ptr_Input_v3uint Input - -// CHECK: [[gl_WorkGrouID:%\d+]] = OpLoad %v3uint %gl_WorkGroupID -// CHECK: [[uint2_GroupID:%\d+]] = OpVectorShuffle %v2uint [[gl_WorkGrouID]] [[gl_WorkGrouID]] 0 1 -// CHECK: OpStore %param_var_tid [[uint2_GroupID]] - -[numthreads(8, 8, 8)] -void main(uint2 tid : SV_GroupID) {} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.int2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.int2.cs.hlsl new file mode 100644 index 000000000..9380e41a7 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.int2.cs.hlsl @@ -0,0 +1,13 @@ +// Run: %dxc -T cs_6_0 -E main + +// CHECK: OpEntryPoint GLCompute %main "main" %gl_LocalInvocationID +// CHECK: OpDecorate %gl_LocalInvocationID BuiltIn LocalInvocationId +// CHECK: %gl_LocalInvocationID = OpVariable %_ptr_Input_v3int Input + +// CHECK: %param_var_gtid = OpVariable %_ptr_Function_v2int Function +// CHECK: [[gl_LocalInvocationID:%\d+]] = OpLoad %v3int %gl_LocalInvocationID +// CHECK: [[int2_GroupThreadID:%\d+]] = OpVectorShuffle %v2int [[gl_LocalInvocationID]] [[gl_LocalInvocationID]] 0 1 +// CHECK: OpStore %param_var_gtid [[int2_GroupThreadID]] + +[numthreads(8, 8, 8)] +void main(int2 gtid : SV_GroupThreadID) {} diff --git a/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.uint2.cs.hlsl b/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.uint2.cs.hlsl deleted file mode 100644 index 7ebc12d27..000000000 --- a/tools/clang/test/CodeGenSPIRV/semantic.group-thread-id.uint2.cs.hlsl +++ /dev/null @@ -1,12 +0,0 @@ -// Run: %dxc -T cs_6_0 -E main - -// CHECK: OpEntryPoint GLCompute %main "main" %gl_LocalInvocationID -// CHECK: OpDecorate %gl_LocalInvocationID BuiltIn LocalInvocationId -// CHECK: %gl_LocalInvocationID = OpVariable %_ptr_Input_v3uint Input - -// CHECK: [[gl_LocalInvocationID:%\d+]] = OpLoad %v3uint %gl_LocalInvocationID -// CHECK: [[uint2_GroupThreadID:%\d+]] = OpVectorShuffle %v2uint [[gl_LocalInvocationID]] [[gl_LocalInvocationID]] 0 1 -// CHECK: OpStore %param_var_gtid [[uint2_GroupThreadID]] - -[numthreads(8, 8, 8)] -void main(uint2 gtid : SV_GroupThreadID) {} diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index 7e28da5ce..62d129ae2 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -501,15 +501,15 @@ TEST_F(FileTest, SemanticDispatchThreadId) { TEST_F(FileTest, SemanticDispatchThreadIdUint) { runFileTest("semantic.dispatch-thread-id.uint.cs.hlsl"); } -TEST_F(FileTest, SemanticDispatchThreadIdUint2) { - runFileTest("semantic.dispatch-thread-id.uint2.cs.hlsl"); +TEST_F(FileTest, SemanticDispatchThreadIdInt2) { + runFileTest("semantic.dispatch-thread-id.int2.cs.hlsl"); } TEST_F(FileTest, SemanticGroupID) { runFileTest("semantic.group-id.cs.hlsl"); } TEST_F(FileTest, SemanticGroupIDUint) { runFileTest("semantic.group-id.uint.cs.hlsl"); } -TEST_F(FileTest, SemanticGroupIDUint2) { - runFileTest("semantic.group-id.uint2.cs.hlsl"); +TEST_F(FileTest, SemanticGroupIDInt2) { + runFileTest("semantic.group-id.int2.cs.hlsl"); } TEST_F(FileTest, SemanticGroupThreadID) { runFileTest("semantic.group-thread-id.cs.hlsl"); @@ -517,8 +517,8 @@ TEST_F(FileTest, SemanticGroupThreadID) { TEST_F(FileTest, SemanticGroupThreadIDUint) { runFileTest("semantic.group-thread-id.uint.cs.hlsl"); } -TEST_F(FileTest, SemanticGroupThreadIDUint2) { - runFileTest("semantic.group-thread-id.uint2.cs.hlsl"); +TEST_F(FileTest, SemanticGroupThreadIDInt2) { + runFileTest("semantic.group-thread-id.int2.cs.hlsl"); } TEST_F(FileTest, SemanticGroupIndex) { runFileTest("semantic.group-index.cs.hlsl"); From d6187949d7995739d908d18f45619e38920199d2 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Thu, 12 Apr 2018 10:52:51 -0700 Subject: [PATCH 04/15] Check input for atan2. (#1213) * Check input for atan2. --- lib/HLSL/HLOperationLower.cpp | 45 ++++++++++++++++++- .../test/CodeGenHLSL/quick-test/atan.hlsl | 13 ++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 tools/clang/test/CodeGenHLSL/quick-test/atan.hlsl diff --git a/lib/HLSL/HLOperationLower.cpp b/lib/HLSL/HLOperationLower.cpp index 7b9d89236..b0266ef7d 100644 --- a/lib/HLSL/HLOperationLower.cpp +++ b/lib/HLSL/HLOperationLower.cpp @@ -1253,7 +1253,50 @@ Value *TranslateAtan2(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode, IRBuilder<> Builder(CI); Value *tan = Builder.CreateFDiv(y, x); - return TrivialDxilUnaryOperation(OP::OpCode::Atan, tan, hlslOP, Builder); + + Value *atan = + TrivialDxilUnaryOperation(OP::OpCode::Atan, tan, hlslOP, Builder); + // TODO: include M_PI from math.h. + const double M_PI = 3.14159265358979323846; + // Modify atan result based on https://en.wikipedia.org/wiki/Atan2. + Type *Ty = x->getType(); + Constant *pi = ConstantFP::get(Ty->getScalarType(), M_PI); + Constant *halfPi = ConstantFP::get(Ty->getScalarType(), M_PI / 2); + Constant *negHalfPi = ConstantFP::get(Ty->getScalarType(), -M_PI / 2); + Constant *zero = ConstantFP::get(Ty->getScalarType(), 0); + if (Ty != Ty->getScalarType()) { + unsigned vecSize = Ty->getVectorNumElements(); + pi = ConstantVector::getSplat(vecSize, pi); + halfPi = ConstantVector::getSplat(vecSize, halfPi); + negHalfPi = ConstantVector::getSplat(vecSize, negHalfPi); + zero = ConstantVector::getSplat(vecSize, zero); + } + Value *atanAddPi = Builder.CreateFAdd(atan, pi); + Value *atanSubPi = Builder.CreateFSub(atan, pi); + + // x > 0 -> atan. + Value *result = atan; + Value *xLt0 = Builder.CreateFCmpOLT(x, zero); + Value *xEq0 = Builder.CreateFCmpOEQ(x, zero); + + Value *yGe0 = Builder.CreateFCmpOGE(y, zero); + Value *yLt0 = Builder.CreateFCmpOLT(y, zero); + // x < 0, y >= 0 -> atan + pi. + Value *xLt0AndyGe0 = Builder.CreateAnd(xLt0, yGe0); + result = Builder.CreateSelect(xLt0AndyGe0, atanAddPi, result); + + // x < 0, y < 0 -> atan - pi. + Value *xLt0AndYLt0 = Builder.CreateAnd(xLt0, yLt0); + result = Builder.CreateSelect(xLt0AndYLt0, atanSubPi, result); + + // x == 0, y < 0 -> -pi/2 + Value *xEq0AndYLt0 = Builder.CreateAnd(xEq0, yLt0); + result = Builder.CreateSelect(xEq0AndYLt0, negHalfPi, result); + // x == 0, y > 0 -> pi/2 + Value *xEq0AndYGe0 = Builder.CreateAnd(xEq0, yGe0); + result = Builder.CreateSelect(xEq0AndYGe0, halfPi, result); + + return result; } Value *TranslateClamp(CallInst *CI, IntrinsicOp IOP, OP::OpCode opcode, diff --git a/tools/clang/test/CodeGenHLSL/quick-test/atan.hlsl b/tools/clang/test/CodeGenHLSL/quick-test/atan.hlsl new file mode 100644 index 000000000..398f92c97 --- /dev/null +++ b/tools/clang/test/CodeGenHLSL/quick-test/atan.hlsl @@ -0,0 +1,13 @@ +// RUN: %dxc -E main -T ps_6_0 %s | FileCheck %s + + +// Make sure there're select for atan2. +// CHECK: Atan +// CHECK: select +// CHECK: select +// CHECK: select +// CHECK: select + +float main(float2 a : A) : SV_Target { + return atan2(a.y, a.x); +} \ No newline at end of file From 6272989a4b54abcfa7a27708efe02c1de7b97032 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 12 Apr 2018 14:59:54 -0400 Subject: [PATCH 05/15] [spirv] Reduce messages printed when testing (#1218) * Stop outputting warning/error messages in tests They were useful initially; but as we become more and more mature and having more and more tests, they are just flooding the output. * Stop outputting info for successful tests --- tools/clang/unittests/SPIRV/FileTestUtils.cpp | 3 +- tools/clang/unittests/SPIRV/TestMain.cpp | 87 +++++++++++++++++-- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/tools/clang/unittests/SPIRV/FileTestUtils.cpp b/tools/clang/unittests/SPIRV/FileTestUtils.cpp index d8a799118..cbb65d8d4 100644 --- a/tools/clang/unittests/SPIRV/FileTestUtils.cpp +++ b/tools/clang/unittests/SPIRV/FileTestUtils.cpp @@ -157,11 +157,10 @@ bool runCompilerWithSpirvGeneration(const llvm::StringRef inputFilePath, // Get compilation results. IFT(pResult->GetStatus(&resultStatus)); - // Get diagnostics string and print warnings and errors to stderr. + // Get diagnostics string. IFT(pResult->GetErrorBuffer(&pErrorBuffer)); const std::string diagnostics((char *)pErrorBuffer->GetBufferPointer(), pErrorBuffer->GetBufferSize()); - fprintf(stderr, "%s\n", diagnostics.c_str()); *errorMessages = diagnostics; if (SUCCEEDED(resultStatus)) { diff --git a/tools/clang/unittests/SPIRV/TestMain.cpp b/tools/clang/unittests/SPIRV/TestMain.cpp index e4f4dc649..c73975bd1 100644 --- a/tools/clang/unittests/SPIRV/TestMain.cpp +++ b/tools/clang/unittests/SPIRV/TestMain.cpp @@ -21,20 +21,87 @@ #endif #endif +namespace { +using namespace ::testing; + +/// A GoogleTest event printer that only prints test failures. +class FailurePrinter : public TestEventListener { +public: + explicit FailurePrinter(TestEventListener *listener) + : defaultListener(listener) {} + + ~FailurePrinter() override { delete defaultListener; } + + void OnTestProgramStart(const UnitTest &ut) override { + defaultListener->OnTestProgramStart(ut); + } + + void OnTestIterationStart(const UnitTest &ut, int iteration) override { + defaultListener->OnTestIterationStart(ut, iteration); + } + + void OnEnvironmentsSetUpStart(const UnitTest &ut) override { + defaultListener->OnEnvironmentsSetUpStart(ut); + } + + void OnEnvironmentsSetUpEnd(const UnitTest &ut) override { + defaultListener->OnEnvironmentsSetUpEnd(ut); + } + + void OnTestCaseStart(const TestCase &tc) override { + defaultListener->OnTestCaseStart(tc); + } + + void OnTestStart(const TestInfo &ti) override { + // Do not output on test start + // defaultListener->OnTestStart(ti); + } + + void OnTestPartResult(const TestPartResult &result) override { + defaultListener->OnTestPartResult(result); + } + + void OnTestEnd(const TestInfo &ti) override { + // Only output if failure on test end + if (ti.result()->Failed()) + defaultListener->OnTestEnd(ti); + } + + void OnTestCaseEnd(const TestCase &tc) override { + defaultListener->OnTestCaseEnd(tc); + } + + void OnEnvironmentsTearDownStart(const UnitTest &ut) override { + defaultListener->OnEnvironmentsTearDownStart(ut); + } + + void OnEnvironmentsTearDownEnd(const UnitTest &ut) override { + defaultListener->OnEnvironmentsTearDownEnd(ut); + } + + void OnTestIterationEnd(const UnitTest &ut, int iteration) override { + defaultListener->OnTestIterationEnd(ut, iteration); + } + + void OnTestProgramEnd(const UnitTest &ut) override { + defaultListener->OnTestProgramEnd(ut); + } + +private: + TestEventListener *defaultListener; +}; +} // namespace + const char *TestMainArgv0; int main(int argc, char **argv) { llvm::sys::PrintStackTraceOnErrorSignal(true /* Disable crash reporting */); - // Initialize both gmock and gtest. - testing::InitGoogleMock(&argc, argv); - for (int i = 1; i < argc; ++i) { if (std::string("--spirv-test-root") == argv[i]) { // Allow the user set the root directory for test input files. if (i + 1 < argc) { - clang::spirv::testOptions::inputDataDir = argv[i + 1]; - i++; + clang::spirv::testOptions::inputDataDir = argv[++i]; } else { fprintf(stderr, "Error: --spirv-test-root requires an argument\n"); return 1; @@ -42,6 +109,16 @@ int main(int argc, char **argv) { } } + // Initialize both gmock and gtest. + testing::InitGoogleMock(&argc, argv); + + // Switch event listener to one that only prints failures. + testing::TestEventListeners &listeners = + ::testing::UnitTest::GetInstance()->listeners(); + auto *defaultPrinter = listeners.Release(listeners.default_result_printer()); + // Google Test takes the ownership. + listeners.Append(new FailurePrinter(defaultPrinter)); + // Make it easy for a test to re-execute itself by saving argv[0]. TestMainArgv0 = argv[0]; From c7b2ba11a300fb92f22d946f44c5c6d1dc17d659 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 12 Apr 2018 15:00:04 -0400 Subject: [PATCH 06/15] [spirv] Emit source file name if -Zi is specified (#1219) --- .../clang/include/clang/SPIRV/EmitSPIRVOptions.h | 1 + tools/clang/include/clang/SPIRV/ModuleBuilder.h | 8 ++++++++ tools/clang/include/clang/SPIRV/Structure.h | 10 +++++++++- tools/clang/lib/SPIRV/SPIRVEmitter.cpp | 15 +++++++++++---- tools/clang/lib/SPIRV/Structure.cpp | 11 +++++++++-- .../test/CodeGenSPIRV/spirv.debug.opsource.hlsl | 9 +++++++++ tools/clang/tools/dxcompiler/dxcompilerobj.cpp | 1 + tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp | 4 ++++ 8 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/spirv.debug.opsource.hlsl diff --git a/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h b/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h index 512df3600..24ce4d040 100644 --- a/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h +++ b/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h @@ -38,6 +38,7 @@ struct EmitSPIRVOptions { bool ignoreUnusedResources; bool enable16BitTypes; bool enableReflect; + bool enableDebugInfo; llvm::StringRef stageIoOrder; llvm::SmallVector bShift; llvm::SmallVector tShift; diff --git a/tools/clang/include/clang/SPIRV/ModuleBuilder.h b/tools/clang/include/clang/SPIRV/ModuleBuilder.h index 40301cc53..4d2bfeedc 100644 --- a/tools/clang/include/clang/SPIRV/ModuleBuilder.h +++ b/tools/clang/include/clang/SPIRV/ModuleBuilder.h @@ -332,6 +332,10 @@ public: inline void setShaderModelVersion(uint32_t major, uint32_t minor); + /// \brief Sets the source file name and the for the OpString for + /// the file name. + inline void setSourceFileName(uint32_t id, std::string name); + /// \brief Adds an execution mode to the module under construction. void addExecutionMode(uint32_t entryPointId, spv::ExecutionMode em, llvm::ArrayRef params); @@ -516,6 +520,10 @@ void ModuleBuilder::setShaderModelVersion(uint32_t major, uint32_t minor) { theModule.setShaderModelVersion(major * 100 + minor * 10); } +void ModuleBuilder::setSourceFileName(uint32_t id, std::string name) { + theModule.setSourceFileName(id, std::move(name)); +} + } // end namespace spirv } // end namespace clang diff --git a/tools/clang/include/clang/SPIRV/Structure.h b/tools/clang/include/clang/SPIRV/Structure.h index 93cf7367c..8fe7cd67b 100644 --- a/tools/clang/include/clang/SPIRV/Structure.h +++ b/tools/clang/include/clang/SPIRV/Structure.h @@ -307,6 +307,7 @@ public: llvm::ArrayRef intefaces); inline void addExecutionMode(Instruction &&); inline void setShaderModelVersion(uint32_t); + inline void setSourceFileName(uint32_t id, std::string name); // TODO: source code debug information inline void addDebugName(uint32_t targetId, llvm::StringRef name, llvm::Optional memberIndex = llvm::None); @@ -338,6 +339,8 @@ private: std::vector entryPoints; std::vector executionModes; uint32_t shaderModelVersion; + uint32_t sourceFileNameId; // The for the OpString for file name + std::string sourceFileName; // TODO: source code debug information std::set debugNames; llvm::SetVector> decorations; @@ -449,7 +452,7 @@ TypeIdPair::TypeIdPair(const Type &ty, uint32_t id) : type(ty), resultId(id) {} SPIRVModule::SPIRVModule() : addressingModel(llvm::None), memoryModel(llvm::None), - shaderModelVersion(0) {} + shaderModelVersion(0), sourceFileNameId(0) {} void SPIRVModule::setVersion(uint32_t version) { header.version = version; } void SPIRVModule::setBound(uint32_t newBound) { header.bound = newBound; } @@ -495,6 +498,11 @@ void SPIRVModule::setShaderModelVersion(uint32_t version) { shaderModelVersion = version; } +void SPIRVModule::setSourceFileName(uint32_t id, std::string name) { + sourceFileNameId = id; + sourceFileName = std::move(name); +} + void SPIRVModule::addDebugName(uint32_t targetId, llvm::StringRef name, llvm::Optional memberIndex) { diff --git a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp index 751b04c1e..8ed94c919 100644 --- a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp +++ b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp @@ -510,7 +510,7 @@ spv::Capability getCapabilityForGroupNonUniform(spv::Op opcode) { return spv::Capability::Max; } -std::string getNamespacePrefix(const Decl* decl) { +std::string getNamespacePrefix(const Decl *decl) { std::string nsPrefix = ""; const DeclContext *dc = decl->getDeclContext(); while (dc && !dc->isTranslationUnit()) { @@ -561,6 +561,16 @@ SPIRVEmitter::SPIRVEmitter(CompilerInstance &ci, EmitSPIRVOptions &options) {}); options.Initialize(); + + // Set shader module version + theBuilder.setShaderModelVersion(shaderModel.GetMajor(), + shaderModel.GetMinor()); + + // Set debug info + const auto &inputFiles = ci.getFrontendOpts().Inputs; + if (options.enableDebugInfo && !inputFiles.empty()) + theBuilder.setSourceFileName(theContext.takeNextId(), + inputFiles.front().getFile().str()); } void SPIRVEmitter::HandleTranslationUnit(ASTContext &context) { @@ -568,9 +578,6 @@ void SPIRVEmitter::HandleTranslationUnit(ASTContext &context) { if (context.getDiagnostics().hasErrorOccurred()) return; - theBuilder.setShaderModelVersion(shaderModel.GetMajor(), - shaderModel.GetMinor()); - TranslationUnitDecl *tu = context.getTranslationUnitDecl(); // The entry function is the seed of the queue. diff --git a/tools/clang/lib/SPIRV/Structure.cpp b/tools/clang/lib/SPIRV/Structure.cpp index 1feba4322..edf055cb8 100644 --- a/tools/clang/lib/SPIRV/Structure.cpp +++ b/tools/clang/lib/SPIRV/Structure.cpp @@ -282,11 +282,18 @@ void SPIRVModule::take(InstBuilder *builder) { consumer(inst.take()); } - if (shaderModelVersion != 0) + if (shaderModelVersion != 0) { + llvm::Optional fileName = llvm::None; + if (!sourceFileName.empty() && sourceFileNameId) { + builder->opString(sourceFileNameId, sourceFileName).x(); + fileName = sourceFileNameId; + } + builder - ->opSource(spv::SourceLanguage::HLSL, shaderModelVersion, llvm::None, + ->opSource(spv::SourceLanguage::HLSL, shaderModelVersion, fileName, llvm::None) .x(); + } // BasicBlock debug names should be emitted only for blocks that are // reachable. diff --git a/tools/clang/test/CodeGenSPIRV/spirv.debug.opsource.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.debug.opsource.hlsl new file mode 100644 index 000000000..d13afad82 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/spirv.debug.opsource.hlsl @@ -0,0 +1,9 @@ +// Run: %dxc -T cs_6_1 -E main -Zi + +// CHECK: [[str:%\d+]] = OpString +// CHECK-SAME: spirv.debug.opsource.hlsl +// CHECK: OpSource HLSL 610 [[str]] + +[numthreads(8, 1, 1)] +void main() { +} diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp index 436f6a336..36ee33d07 100644 --- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp +++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp @@ -482,6 +482,7 @@ public: spirvOpts.allowedExtensions = opts.SpvExtensions; spirvOpts.targetEnv = opts.SpvTargetEnv; spirvOpts.enable16BitTypes = opts.Enable16BitTypes; + spirvOpts.enableDebugInfo = opts.DebugInfo; clang::EmitSPIRVAction action(spirvOpts); FrontendInputFile file(utf8SourceName.m_psz, IK_HLSL); action.BeginSourceFile(compiler, file); diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index 62d129ae2..bcf46ea9f 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -1262,6 +1262,10 @@ TEST_F(FileTest, SpirvLegalizationTextureBuffer) { /*runValidation=*/false); } +TEST_F(FileTest, SpirvDebugOpSource) { + runFileTest("spirv.debug.opsource.hlsl"); +} + TEST_F(FileTest, VulkanAttributeErrors) { runFileTest("vk.attribute.error.hlsl", Expect::Failure); } From b241b8bd9887bcf60eddd76aac588a29a68db242 Mon Sep 17 00:00:00 2001 From: Ehsan Date: Thu, 12 Apr 2018 18:54:08 -0400 Subject: [PATCH 07/15] [spirv] Handle rvalue passed to Interlocked methods (#1220) --- tools/clang/lib/SPIRV/SPIRVEmitter.cpp | 9 ++++++++- .../intrinsics.interlocked-methods.cs.hlsl | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp index 8ed94c919..684b26ef9 100644 --- a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp +++ b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp @@ -6554,7 +6554,14 @@ SPIRVEmitter::processIntrinsicInterlockedMethod(const CallExpr *expr, if (isBufferTextureIndexing(callExpr, &base, &index)) { const auto ptrType = theBuilder.getPointerType(baseTypeId, spv::StorageClass::Image); - const auto baseId = doExpr(base); + auto baseId = doExpr(base); + if (baseId.isRValue()) { + // OpImageTexelPointer's Image argument must have a type of + // OpTypePointer with Type OpTypeImage. Need to create a temporary + // variable if the baseId is an rvalue. + baseId = createTemporaryVar( + base->getType(), TypeTranslator::getName(base->getType()), baseId); + } const auto coordId = doExpr(index); ptr = theBuilder.createImageTexelPointer(ptrType, baseId, coordId, zero); } diff --git a/tools/clang/test/CodeGenSPIRV/intrinsics.interlocked-methods.cs.hlsl b/tools/clang/test/CodeGenSPIRV/intrinsics.interlocked-methods.cs.hlsl index 93cf8234a..9071f084f 100644 --- a/tools/clang/test/CodeGenSPIRV/intrinsics.interlocked-methods.cs.hlsl +++ b/tools/clang/test/CodeGenSPIRV/intrinsics.interlocked-methods.cs.hlsl @@ -3,6 +3,11 @@ groupshared int dest_i; groupshared uint dest_u; +RWBuffer buff; +RWBuffer getDest() { + return buff; +} + void main() { uint original_u_val; @@ -21,6 +26,11 @@ void main() // CHECK-NEXT: OpStore %original_i_val [[iadd27]] InterlockedAdd(dest_i, val1, original_i_val); +// CHECK: [[buff:%\d+]] = OpFunctionCall %type_buffer_image %getDest +// CHECK-NEXT: OpStore %temp_var_RWBuffer [[buff]] +// CHECK-NEXT: OpImageTexelPointer %_ptr_Image_uint %temp_var_RWBuffer %uint_0 %uint_0 + InterlockedAdd(getDest()[0], val1, original_i_val); + // CHECK: [[and28:%\d+]] = OpAtomicAnd %uint %dest_u %uint_1 %uint_0 %uint_10 // CHECK-NEXT: OpStore %original_u_val [[and28]] InterlockedAnd(dest_u, 10, original_u_val); From bdd9254a82a5a97fe663e29eb276c9e22cd3954b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 13 Apr 2018 15:00:40 -0400 Subject: [PATCH 08/15] [spirv] Fix push constant defined from anonymous struct (#1223) When skipping decls not externally visible, we should make sure that they are in the translation unit or a namespace, which means they are for $Globals cbuffer. For other cases, we should not skip. Fixes https://github.com/Microsoft/DirectXShaderCompiler/issues/1221 --- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 7 ++- tools/clang/lib/SPIRV/TypeTranslator.cpp | 51 +++++++++++-------- .../clang/test/CodeGenSPIRV/type.struct.hlsl | 6 +-- .../vk.push-constant.anon-struct.hlsl | 25 +++++++++ .../unittests/SPIRV/CodeGenSPIRVTest.cpp | 3 ++ 5 files changed, 65 insertions(+), 27 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.push-constant.anon-struct.hlsl diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index dd156bb23..6e1ca5664 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -471,11 +471,10 @@ uint32_t DeclResultIdMapper::createStructOrStructArrayVarOfExplicitLayout( llvm::StringRef varName) { // cbuffers are translated into OpTypeStruct with Block decoration. // tbuffers are translated into OpTypeStruct with BufferBlock decoration. - // PushConstants are translated into OpTypeStruct with Block decoration. + // Push constants are translated into OpTypeStruct with Block decoration. // - // Both cbuffers and tbuffers have the SPIR-V Uniform storage class. cbuffers - // follow GLSL std140 layout rules, and tbuffers follow GLSL std430 layout - // rules. PushConstants follow GLSL std430 layout rules. + // Both cbuffers and tbuffers have the SPIR-V Uniform storage class. + // Push constants have the SPIR-V PushConstant storage class. const bool forCBuffer = usageKind == ContextUsageKind::CBuffer; const bool forTBuffer = usageKind == ContextUsageKind::TBuffer; diff --git a/tools/clang/lib/SPIRV/TypeTranslator.cpp b/tools/clang/lib/SPIRV/TypeTranslator.cpp index 609aad39a..f97a29080 100644 --- a/tools/clang/lib/SPIRV/TypeTranslator.cpp +++ b/tools/clang/lib/SPIRV/TypeTranslator.cpp @@ -1133,40 +1133,51 @@ TypeTranslator::getCapabilityForStorageImageReadWrite(QualType type) { bool TypeTranslator::shouldSkipInStructLayout(const Decl *decl) { // Ignore implicit generated struct declarations/constructors/destructors - // Ignore embedded type decls - // Ignore embeded function decls - // Ignore empty decls - if (decl->isImplicit() || isa(decl) || isa(decl) || - isa(decl)) + if (decl->isImplicit()) + return true; + // Ignore embedded type decls + if (isa(decl)) + return true; + // Ignore embeded function decls + if (isa(decl)) + return true; + // Ignore empty decls + if (isa(decl)) return true; - - // For $Globals (whose "struct" is the TranslationUnit) - // Ignore resources in the TranslationUnit "struct" // For the $Globals cbuffer, we only care about externally-visiable // non-resource-type variables. The rest should be filtered out. + const auto *declContext = decl->getDeclContext(); + // Special check for ConstantBuffer/TextureBuffer, whose DeclContext is a // HLSLBufferDecl. So that we need to check the HLSLBufferDecl's parent decl // to check whether this is a ConstantBuffer/TextureBuffer defined in the // global namespace. + // Note that we should not be seeing ConstantBuffer/TextureBuffer for normal + // cbuffer/tbuffer or push constant blocks. So this case should only happen + // for $Globals cbuffer. if (isConstantTextureBuffer(decl) && - decl->getDeclContext()->getLexicalParent()->isTranslationUnit()) + declContext->getLexicalParent()->isTranslationUnit()) return true; - // External visibility - if (const auto *declDecl = dyn_cast(decl)) - if (!declDecl->hasExternalFormalLinkage()) + // $Globals' "struct" is the TranslationUnit, so we should ignore resources + // in the TranslationUnit "struct" and its child namespaces. + if (declContext->isTranslationUnit() || declContext->isNamespace()) { + // External visibility + if (const auto *declDecl = dyn_cast(decl)) + if (!declDecl->hasExternalFormalLinkage()) + return true; + + // cbuffer/tbuffer + if (isa(decl)) return true; - // cbuffer/tbuffer - if (isa(decl)) - return true; - - // Other resource types - if (const auto *valueDecl = dyn_cast(decl)) - if (isResourceType(valueDecl)) - return true; + // Other resource types + if (const auto *valueDecl = dyn_cast(decl)) + if (isResourceType(valueDecl)) + return true; + } return false; } diff --git a/tools/clang/test/CodeGenSPIRV/type.struct.hlsl b/tools/clang/test/CodeGenSPIRV/type.struct.hlsl index 885c543e7..388dc0abb 100644 --- a/tools/clang/test/CodeGenSPIRV/type.struct.hlsl +++ b/tools/clang/test/CodeGenSPIRV/type.struct.hlsl @@ -57,13 +57,13 @@ void main() { S s; T t; -// CHECK: %_ptr_Function__struct_[[num]] = OpTypePointer Function %_struct_[[num]] +// CHECK: %R = OpTypeStruct %v2float -// CHECK: %r0 = OpVariable %_ptr_Function__struct_[[num]] Function +// CHECK: %r0 = OpVariable %_ptr_Function_R Function struct R { float2 rVal; } r0; -// CHECK: %r1 = OpVariable %_ptr_Function__struct_[[num]] Function +// CHECK: %r1 = OpVariable %_ptr_Function_R Function R r1; } diff --git a/tools/clang/test/CodeGenSPIRV/vk.push-constant.anon-struct.hlsl b/tools/clang/test/CodeGenSPIRV/vk.push-constant.anon-struct.hlsl new file mode 100644 index 000000000..6c4614035 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.push-constant.anon-struct.hlsl @@ -0,0 +1,25 @@ +// Run: %dxc -T vs_6_0 -E main + +// CHECK: OpName %type_PushConstant_ "type.PushConstant." +// CHECK: OpMemberName %type_PushConstant_ 0 "a" +// CHECK: OpMemberName %type_PushConstant_ 1 "b" +// CHECK: OpMemberName %type_PushConstant_ 2 "c" + +// CHECK: %type_PushConstant_ = OpTypeStruct %int %float %v3float +// CHECK: %_ptr_PushConstant_type_PushConstant_ = OpTypePointer PushConstant %type_PushConstant_ +[[vk::push_constant]] +struct { + int a; + float b; + float3 c; +} +// CHECK: %PushConstants = OpVariable %_ptr_PushConstant_type_PushConstant_ PushConstant +PushConstants; + +RWBuffer Output; + +[numthreads(1, 1, 1)] +void main() { +// CHECK: OpAccessChain %_ptr_PushConstant_int %PushConstants %int_0 + Output[0] = PushConstants.a; +} diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index bcf46ea9f..775d7e68d 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -1335,6 +1335,9 @@ TEST_F(FileTest, VulkanPushConstant) { runFileTest("vk.push-constant.hlsl"); } TEST_F(FileTest, VulkanPushConstantOffset) { runFileTest("vk.push-constant.offset.hlsl"); } +TEST_F(FileTest, VulkanPushConstantAnonymousStruct) { + runFileTest("vk.push-constant.anon-struct.hlsl"); +} TEST_F(FileTest, VulkanMultiplePushConstant) { runFileTest("vk.push-constant.multiple.hlsl", Expect::Failure); } From 35d0aeae20a18ddc7d1f5a1b09b68f816c5de8d9 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 13 Apr 2018 15:29:57 -0400 Subject: [PATCH 09/15] Change Appveyor to do Release build (#1200) Also changed to use the build project from antiagainst's account. --- README.md | 4 +++- appveyor.yml | 4 ++-- tools/clang/test/CodeGenHLSL/quick-test/convergent.hlsl | 3 ++- utils/appveyor/appveyor_test.ps1 | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c47ed28a0..549242845 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # DirectX Shader Compiler -[![Build status](https://ci.appveyor.com/api/projects/status/5cwy3b8y1oi71lvl?svg=true)](https://ci.appveyor.com/project/marcelolr/directxshadercompiler) +[![Build status](https://ci.appveyor.com/api/projects/status/oaf66n7w30xbrg38/branch/master?svg=true)](https://ci.appveyor.com/project/antiagainst/directxshadercompiler/branch/master) The DirectX Shader Compiler project includes a compiler and related tools used to compile High-Level Shader Language (HLSL) programs into DirectX Intermediate Language (DXIL) representation. Applications that make use of DirectX for graphics, games, and computation can use it to generate shader programs. @@ -28,6 +28,8 @@ As an example of community contribution, this project can also target the [SPIR- ## Building Sources +Note: Instead of building manually, you can download the artifacts built by Appveyor for the latest master branch at [here](https://ci.appveyor.com/project/antiagainst/directxshadercompiler/branch/master/artifacts). + Before you build, you will need to have some additional software installed. This is the most straightforward path - see [Building Sources](https://github.com/Microsoft/DirectXShaderCompiler/wiki/Building-Sources) on the Wiki for more options, including Visual Studio 2015 and Ninja support. * [Git](http://git-scm.com/downloads). diff --git a/appveyor.yml b/appveyor.yml index 98e925ead..e1d71c405 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,7 +2,7 @@ version: 1.0.{build} image: Visual Studio 2017 platform: x64 -configuration: Debug +configuration: Release clone_folder: c:\projects\DirectXShaderCompiler clone_depth: 1 @@ -22,7 +22,7 @@ build_script: test_script: - ps: utils\appveyor\appveyor_test.ps1 -- cmd: call utils\hct\hcttest spirv_only +- cmd: call utils\hct\hcttest -rel spirv_only after_test: - cmd: cd build\%CONFIGURATION%\bin diff --git a/tools/clang/test/CodeGenHLSL/quick-test/convergent.hlsl b/tools/clang/test/CodeGenHLSL/quick-test/convergent.hlsl index 7849f9114..fb0b389c6 100644 --- a/tools/clang/test/CodeGenHLSL/quick-test/convergent.hlsl +++ b/tools/clang/test/CodeGenHLSL/quick-test/convergent.hlsl @@ -3,7 +3,8 @@ // Make sure add is not sink into if. // CHECK: fadd // CHECK: fadd -// CHECK: if.then +// CHECK: fcmp +// CHECK-NEXT: br Texture2D tex; SamplerState s; diff --git a/utils/appveyor/appveyor_test.ps1 b/utils/appveyor/appveyor_test.ps1 index 21cf8acdf..0b10f7a4c 100644 --- a/utils/appveyor/appveyor_test.ps1 +++ b/utils/appveyor/appveyor_test.ps1 @@ -39,7 +39,7 @@ function Invoke-AppveyorTestsRestMethod($appveyorTests) { } function Invoke-TE($logfile) { - $testdll = "$env:HLSL_BLD_DIR\Debug\bin\clang-hlsl-tests.dll" + $testdll = "$env:HLSL_BLD_DIR\Release\bin\clang-hlsl-tests.dll" $p = Start-Process "te.exe" -Args "$testdll /logOutput:Low /logFile:$logfile /enableWttLogging /p:HlslDataDir=%HLSL_SRC_DIR%\tools\clang\test\HLSL /labMode /miniDumpOnCrash" -Wait -NoNewWindow -PassThru return $p.ExitCode } From a4491dd4398ea018e517beaf9027a45bcf622b86 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 13 Apr 2018 15:40:44 -0400 Subject: [PATCH 10/15] [spirv] Support shifting all sets with -fvk-*-shift N all (#1224) --- docs/SPIR-V.rst | 5 +- include/dxc/Support/HLSLOptions.h | 8 +-- lib/DxcSupport/HLSLOptions.cpp | 53 ++++++++++++------- .../include/clang/SPIRV/EmitSPIRVOptions.h | 8 +-- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 14 +++-- .../CodeGenSPIRV/vk.binding.cl.all-sets.hlsl | 53 +++++++++++++++++++ .../unittests/SPIRV/CodeGenSPIRVTest.cpp | 7 ++- 7 files changed, 114 insertions(+), 34 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/vk.binding.cl.all-sets.hlsl diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index 43ad17e34..ab42bb6a7 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2683,8 +2683,9 @@ codegen for Vulkan: sets its Vulkan descriptor set to ``M`` and binding number to ``X + N``. If you need to shift the inferred binding numbers for more than one space, provide more than one such option. If more than one such option is provided - for the same space, the last one takes effect. See `HLSL register and Vulkan - binding`_ for explanation and examples. + for the same space, the last one takes effect. If you need to shift the + inferred binding numbers for all sets, use ``all`` as ``M``. + See `HLSL register and Vulkan binding`_ for explanation and examples. - ``-fvk-t-shift N M``, similar to ``-fvk-b-shift``, but for t-type registers. - ``-fvk-s-shift N M``, similar to ``-fvk-b-shift``, but for s-type registers. - ``-fvk-u-shift N M``, similar to ``-fvk-b-shift``, but for u-type registers. diff --git a/include/dxc/Support/HLSLOptions.h b/include/dxc/Support/HLSLOptions.h index 1ee92c150..602e5592a 100644 --- a/include/dxc/Support/HLSLOptions.h +++ b/include/dxc/Support/HLSLOptions.h @@ -167,10 +167,10 @@ public: bool VkUseDxLayout; // OPT_fvk_use_dx_layout bool SpvEnableReflect; // OPT_fspv_reflect llvm::StringRef VkStageIoOrder; // OPT_fvk_stage_io_order - llvm::SmallVector VkBShift; // OPT_fvk_b_shift - llvm::SmallVector VkTShift; // OPT_fvk_t_shift - llvm::SmallVector VkSShift; // OPT_fvk_s_shift - llvm::SmallVector VkUShift; // OPT_fvk_u_shift + llvm::SmallVector VkBShift; // OPT_fvk_b_shift + llvm::SmallVector VkTShift; // OPT_fvk_t_shift + llvm::SmallVector VkSShift; // OPT_fvk_s_shift + llvm::SmallVector VkUShift; // OPT_fvk_u_shift llvm::SmallVector SpvExtensions; // OPT_fspv_extension llvm::StringRef SpvTargetEnv; // OPT_fspv_target_env #endif diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index a78c87a1a..2706612ed 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -490,26 +490,43 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, opts.VkIgnoreUnusedResources = Args.hasFlag(OPT_fvk_ignore_unused_resources, OPT_INVALID, false); // Collects the arguments for -fvk-{b|s|t|u}-shift. - const auto handleVkShiftArgs = [genSpirv, &Args, &errors]( - OptSpecifier id, const char* name, llvm::SmallVectorImpl* shifts) { - const auto values = Args.getAllArgValues(id); + const auto handleVkShiftArgs = + [genSpirv, &Args, &errors](OptSpecifier id, const char *name, + llvm::SmallVectorImpl *shifts) { + const auto values = Args.getAllArgValues(id); - if (!genSpirv && !values.empty()) { - errors << "-fvk-" << name << "-shift requires -spirv"; - return false; - } + if (!genSpirv && !values.empty()) { + errors << "-fvk-" << name << "-shift requires -spirv"; + return false; + } - shifts->clear(); - for (const auto& val : values) { - uint32_t number = 0; - if (llvm::StringRef(val).getAsInteger(10, number)) { - errors << "invalid -fvk-" << name << "-shift argument: " << val; - return false; - } - shifts->push_back(number); - } - return true; - }; + shifts->clear(); + bool setForAll = false; + + for (const auto &val : values) { + int32_t number = 0; + if (val == "all") { + number = -1; + setForAll = true; + } else { + if (llvm::StringRef(val).getAsInteger(10, number)) { + errors << "invalid -fvk-" << name << "-shift argument: " << val; + return false; + } + if (number < 0) { + errors << "negative -fvk-" << name << "-shift argument: " << val; + return false; + } + } + shifts->push_back(number); + } + if (setForAll && shifts->size() > 2) { + errors << "setting all sets via -fvk-" << name + << "-shift argument should be used alone"; + return false; + } + return true; + }; if (!handleVkShiftArgs(OPT_fvk_b_shift, "b", &opts.VkBShift) || !handleVkShiftArgs(OPT_fvk_t_shift, "t", &opts.VkTShift) || diff --git a/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h b/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h index 24ce4d040..d654be3e2 100644 --- a/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h +++ b/tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h @@ -40,10 +40,10 @@ struct EmitSPIRVOptions { bool enableReflect; bool enableDebugInfo; llvm::StringRef stageIoOrder; - llvm::SmallVector bShift; - llvm::SmallVector tShift; - llvm::SmallVector sShift; - llvm::SmallVector uShift; + llvm::SmallVector bShift; + llvm::SmallVector tShift; + llvm::SmallVector sShift; + llvm::SmallVector uShift; llvm::SmallVector allowedExtensions; llvm::StringRef targetEnv; spirv::LayoutRule cBufferLayoutRule; diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 6e1ca5664..9a1d222fe 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1015,15 +1015,19 @@ namespace { /// sets. class BindingShiftMapper { public: - explicit BindingShiftMapper(const llvm::SmallVectorImpl &shifts) + explicit BindingShiftMapper(const llvm::SmallVectorImpl &shifts) : masterShift(0) { assert(shifts.size() % 2 == 0); - for (uint32_t i = 0; i < shifts.size(); i += 2) - perSetShift[shifts[i + 1]] = shifts[i]; + if (shifts.size() == 2 && shifts[1] == -1) { + masterShift = shifts[0]; + } else { + for (uint32_t i = 0; i < shifts.size(); i += 2) + perSetShift[shifts[i + 1]] = shifts[i]; + } } /// Returns the shift amount for the given set. - uint32_t getShiftForSet(uint32_t set) const { + int32_t getShiftForSet(int32_t set) const { const auto found = perSetShift.find(set); if (found != perSetShift.end()) return found->second; @@ -1032,7 +1036,7 @@ public: private: uint32_t masterShift; /// Shift amount applies to all sets. - llvm::DenseMap perSetShift; + llvm::DenseMap perSetShift; }; } // namespace diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.cl.all-sets.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.all-sets.hlsl new file mode 100644 index 000000000..ae89a4aa3 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/vk.binding.cl.all-sets.hlsl @@ -0,0 +1,53 @@ +// Run: %dxc -T ps_6_0 -E main -fvk-b-shift 1000 all -fvk-t-shift 2000 all -fvk-s-shift 3000 all -fvk-u-shift 4000 all + +struct S { + float4 f; +}; + +// Explicit binding assignment is unaffected. + +// CHECK: OpDecorate %cbuffer3 DescriptorSet 0 +// CHECK: OpDecorate %cbuffer3 Binding 42 +[[vk::binding(42)]] +ConstantBuffer cbuffer3 : register(b10, space2); + +// CHECK: OpDecorate %cbuffer1 DescriptorSet 0 +// CHECK: OpDecorate %cbuffer1 Binding 1000 +ConstantBuffer cbuffer1 : register(b0); +// CHECK: OpDecorate %cbuffer2 DescriptorSet 2 +// CHECK: OpDecorate %cbuffer2 Binding 1000 +ConstantBuffer cbuffer2 : register(b0, space2); + +// CHECK: OpDecorate %texture1 DescriptorSet 1 +// CHECK: OpDecorate %texture1 Binding 2001 +Texture2D texture1: register(t1, space1); +// CHECK: OpDecorate %texture2 DescriptorSet 0 +// CHECK: OpDecorate %texture2 Binding 2001 +Texture2D texture2: register(t1); + +// CHECK: OpDecorate %sampler1 DescriptorSet 0 +// CHECK: OpDecorate %sampler1 Binding 3000 +// CHECK: OpDecorate %sampler2 DescriptorSet 2 +// CHECK: OpDecorate %sampler2 Binding 3000 +SamplerState sampler1: register(s0); +SamplerState sampler2: register(s0, space2); + +// CHECK: OpDecorate %rwbuffer1 DescriptorSet 3 +// CHECK: OpDecorate %rwbuffer1 Binding 4003 +RWBuffer rwbuffer1 : register(u3, space3); +// CHECK: OpDecorate %rwbuffer2 DescriptorSet 0 +// CHECK: OpDecorate %rwbuffer2 Binding 4003 +RWBuffer rwbuffer2 : register(u3); + +// Lacking binding assignment is unaffacted. + +// CHECK: OpDecorate %cbuffer4 DescriptorSet 0 +// CHECK: OpDecorate %cbuffer4 Binding 0 +ConstantBuffer cbuffer4; +// CHECK: OpDecorate %cbuffer5 DescriptorSet 0 +// CHECK: OpDecorate %cbuffer5 Binding 1 +ConstantBuffer cbuffer5; + +float4 main() : SV_Target { + return cbuffer1.f; +} diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index 775d7e68d..60765f26b 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -1321,10 +1321,15 @@ TEST_F(FileTest, VulkanRegisterBinding) { runFileTest("vk.binding.register.hlsl"); } TEST_F(FileTest, VulkanRegisterBindingShift) { - // Resource binding from :register() and with shift specified via + // Resource binding from :register() with shift specified via // command line option runFileTest("vk.binding.cl.hlsl"); } +TEST_F(FileTest, VulkanRegisterBindingShiftAllSets) { + // Resource binding from :register() with shift specified for all sets via + // command line option + runFileTest("vk.binding.cl.all-sets.hlsl"); +} TEST_F(FileTest, VulkanStructuredBufferCounter) { // [[vk::counter_binding()]] for RWStructuredBuffer, AppendStructuredBuffer, // and ConsumeStructuredBuffer From 483901edbae750b5b9b59a669458d5aad1980aec Mon Sep 17 00:00:00 2001 From: Ehsan Date: Fri, 13 Apr 2018 18:14:35 -0400 Subject: [PATCH 11/15] [spirv] Member fn call use Function storage class. (#1226) Fixes https://github.com/Microsoft/DirectXShaderCompiler/issues/1222 --- tools/clang/lib/SPIRV/SPIRVEmitter.cpp | 26 +++++++------------ .../test/CodeGenSPIRV/oo.struct.method.hlsl | 24 ++++++++++++++--- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp index 684b26ef9..68b4c02a1 100644 --- a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp +++ b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp @@ -1957,7 +1957,7 @@ SpirvEvalInfo SPIRVEmitter::processCall(const CallExpr *callExpr) { bool isNonStaticMemberCall = false; QualType objectType = {}; // Type of the object (if exists) SpirvEvalInfo objectEvalInfo = 0; // EvalInfo for the object (if exists) - bool objectNeedsTempVar = false; // Temporary variable for lvalue object + bool needsTempVar = false; // Whether we need temporary variable. llvm::SmallVector params; // Temporary variables llvm::SmallVector args; // Evaluated arguments @@ -1982,17 +1982,11 @@ SpirvEvalInfo SPIRVEmitter::processCall(const CallExpr *callExpr) { // If not already a variable, we need to create a temporary variable and // pass the object pointer to the function. Example: // getObject().objectMethod(); - bool needsTempVar = objectEvalInfo.isRValue(); - - // Try to see if we are calling methods on a global variable, which is put - // in the Private storage class. We also need to create temporary variable - // for it since the function signature expects all arguments in the - // Function storage class. - if (!needsTempVar) - if (const auto *declRefExpr = dyn_cast(object)) - if (const auto *refDecl = declRefExpr->getFoundDecl()) - if (const auto *varDecl = dyn_cast(refDecl)) - needsTempVar = objectNeedsTempVar = varDecl->hasGlobalStorage(); + // Also, any parameter passed to the member function must be of Function + // storage class. + needsTempVar = + objectEvalInfo.isRValue() || + objectEvalInfo.getStorageClass() != spv::StorageClass::Function; if (needsTempVar) { objectId = @@ -2048,10 +2042,10 @@ SpirvEvalInfo SPIRVEmitter::processCall(const CallExpr *callExpr) { const uint32_t retVal = theBuilder.createFunctionCall(retType, funcId, params); - // If we created a temporary variable for the object this method is invoked - // upon, we need to copy the contents in the temporary variable back to the - // original object's variable in case there are side effects. - if (objectNeedsTempVar) { + // If we created a temporary variable for the lvalue object this method is + // invoked upon, we need to copy the contents in the temporary variable back + // to the original object's variable in case there are side effects. + if (needsTempVar && !objectEvalInfo.isRValue()) { const uint32_t typeId = typeTranslator.translateType(objectType); const uint32_t value = theBuilder.createLoad(typeId, params.front()); storeValue(objectEvalInfo, value, objectType); diff --git a/tools/clang/test/CodeGenSPIRV/oo.struct.method.hlsl b/tools/clang/test/CodeGenSPIRV/oo.struct.method.hlsl index 08c6940ca..f0b90f444 100644 --- a/tools/clang/test/CodeGenSPIRV/oo.struct.method.hlsl +++ b/tools/clang/test/CodeGenSPIRV/oo.struct.method.hlsl @@ -59,6 +59,12 @@ T foo() { return t; } +struct R { + int a; + void incr() { ++a; } +}; +RWStructuredBuffer rwsb; + // CHECK: [[ft_f32:%\d+]] = OpTypeFunction %float // CHECK: [[ft_S:%\d+]] = OpTypeFunction %float %_ptr_Function_S // CHECK: [[ft_S_f32:%\d+]] = OpTypeFunction %float %_ptr_Function_S %_ptr_Function_float @@ -98,6 +104,18 @@ float main() : A { // CHECK-NEXT: {{%\d+}} = OpFunctionCall %float %S_fn_ref %temp_var_S float f2 = foo().get_S().fn_ref(); +// CHECK: [[uniformPtr:%\d+]] = OpAccessChain %_ptr_Uniform_R %rwsb %int_0 %uint_0 +// CHECK-NEXT: [[originalObj:%\d+]] = OpLoad %R [[uniformPtr]] +// CHECK-NEXT: [[member:%\d+]] = OpCompositeExtract %int [[originalObj]] 0 +// CHECK-NEXT: [[tempVar:%\d+]] = OpCompositeConstruct %R_0 [[member]] +// CHECK-NEXT: OpStore %temp_var_R [[tempVar]] +// CHECK-NEXT: OpFunctionCall %void %R_incr %temp_var_R +// CHECK-NEXT: [[tempVar:%\d+]] = OpLoad %R_0 %temp_var_R +// CHECK-NEXT: [[tempVarMember:%\d+]] = OpCompositeExtract %int [[tempVar]] 0 +// CHECK-NEXT: [[newR:%\d+]] = OpCompositeConstruct %R [[tempVarMember]] +// CHECK-NEXT: OpStore [[uniformPtr]] [[newR]] + rwsb[0].incr(); + return f1; // CHECK: OpFunctionEnd } @@ -151,8 +169,8 @@ float main() : A { // CHECK: OpFunctionEnd // CHECK: %S_fn_param = OpFunction %float None [[ft_S_f32]] -// CHECK-NEXT: %param_this_4 = OpFunctionParameter %_ptr_Function_S +// CHECK-NEXT: %param_this_5 = OpFunctionParameter %_ptr_Function_S // CHECK-NEXT: %c_0 = OpFunctionParameter %_ptr_Function_float -// CHECK-NEXT: %bb_entry_8 = OpLabel -// CHECK: {{%\d+}} = OpAccessChain %_ptr_Function_float %param_this_4 %int_0 +// CHECK-NEXT: OpLabel +// CHECK: OpAccessChain %_ptr_Function_float %param_this_5 %int_0 // CHECK: OpFunctionEnd From c2993ddf37d96e9ce8f43513a0f595fc2b4fc707 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 16 Apr 2018 10:19:51 -0400 Subject: [PATCH 12/15] [spirv] Stop emitting SPV_GOOGLE_decorate_string (#1229) Its functionality is subsumed by SPV_GOOGLE_hlsl_functionality1. See https://github.com/KhronosGroup/SPIRV-Registry/pull/4 --- external/SPIRV-Headers | 2 +- tools/clang/include/clang/SPIRV/FeatureManager.h | 1 - tools/clang/lib/SPIRV/FeatureManager.cpp | 6 ------ tools/clang/lib/SPIRV/ModuleBuilder.cpp | 1 - tools/clang/test/CodeGenSPIRV/spirv.interface.ds.hlsl | 1 - tools/clang/test/CodeGenSPIRV/spirv.interface.gs.hlsl | 1 - tools/clang/test/CodeGenSPIRV/spirv.interface.hs.hlsl | 1 - tools/clang/test/CodeGenSPIRV/spirv.interface.ps.hlsl | 1 - tools/clang/test/CodeGenSPIRV/spirv.interface.vs.hlsl | 1 - 9 files changed, 1 insertion(+), 14 deletions(-) diff --git a/external/SPIRV-Headers b/external/SPIRV-Headers index 12f8de9f0..3a4dbdde9 160000 --- a/external/SPIRV-Headers +++ b/external/SPIRV-Headers @@ -1 +1 @@ -Subproject commit 12f8de9f04327336b699b1b80aa390ae7f9ddbf4 +Subproject commit 3a4dbdde9a9b2cf23736694ba70262dce27fbeaa diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 74ea44626..9c4567933 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -38,7 +38,6 @@ enum class Extension { EXT_shader_stencil_export, AMD_gpu_shader_half_float, AMD_shader_explicit_vertex_parameter, - GOOGLE_decorate_string, GOOGLE_hlsl_functionality1, Unknown, }; diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index 807a20e4e..ef03e6b31 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -61,9 +61,6 @@ bool FeatureManager::allowExtension(llvm::StringRef name) { } allowedExtensions.set(static_cast(symbol)); - if (symbol == Extension::GOOGLE_hlsl_functionality1) - allowedExtensions.set( - static_cast(Extension::GOOGLE_decorate_string)); return true; } @@ -109,7 +106,6 @@ Extension FeatureManager::getExtensionSymbol(llvm::StringRef name) { Extension::AMD_gpu_shader_half_float) .Case("SPV_AMD_shader_explicit_vertex_parameter", Extension::AMD_shader_explicit_vertex_parameter) - .Case("SPV_GOOGLE_decorate_string", Extension::GOOGLE_decorate_string) .Case("SPV_GOOGLE_hlsl_functionality1", Extension::GOOGLE_hlsl_functionality1) .Default(Extension::Unknown); @@ -133,8 +129,6 @@ const char *FeatureManager::getExtensionName(Extension symbol) { return "SPV_AMD_gpu_shader_half_float"; case Extension::AMD_shader_explicit_vertex_parameter: return "SPV_AMD_shader_explicit_vertex_parameter"; - case Extension::GOOGLE_decorate_string: - return "SPV_GOOGLE_decorate_string"; case Extension::GOOGLE_hlsl_functionality1: return "SPV_GOOGLE_hlsl_functionality1"; default: diff --git a/tools/clang/lib/SPIRV/ModuleBuilder.cpp b/tools/clang/lib/SPIRV/ModuleBuilder.cpp index 76cf708b5..c15b164c9 100644 --- a/tools/clang/lib/SPIRV/ModuleBuilder.cpp +++ b/tools/clang/lib/SPIRV/ModuleBuilder.cpp @@ -845,7 +845,6 @@ void ModuleBuilder::decorateHlslSemantic(uint32_t targetId, llvm::StringRef semantic, llvm::Optional memberIdx) { if (allowReflect) { - addExtension(Extension::GOOGLE_decorate_string, "SPIR-V reflection", {}); addExtension(Extension::GOOGLE_hlsl_functionality1, "SPIR-V reflection", {}); theModule.addDecoration( diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.ds.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.ds.hlsl index 973ed4606..20c1effdd 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.interface.ds.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.interface.ds.hlsl @@ -4,7 +4,6 @@ // CHECK: OpCapability CullDistance // CHECK: OpCapability Tessellation -// CHECK: OpExtension "SPV_GOOGLE_decorate_string" // CHECK: OpExtension "SPV_GOOGLE_hlsl_functionality1" // HS PCF output diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.gs.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.gs.hlsl index fd99d0eb2..2c6050c35 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.interface.gs.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.interface.gs.hlsl @@ -4,7 +4,6 @@ // CHECK: OpCapability CullDistance // CHECK: OpCapability Geometry -// CHECK: OpExtension "SPV_GOOGLE_decorate_string" // CHECK: OpExtension "SPV_GOOGLE_hlsl_functionality1" struct GsPerVertexIn { diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.hs.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.hs.hlsl index d0866f0f8..614409b8f 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.interface.hs.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.interface.hs.hlsl @@ -6,7 +6,6 @@ // CHECK: OpCapability CullDistance // CHECK: OpCapability Tessellation -// CHECK: OpExtension "SPV_GOOGLE_decorate_string" // CHECK: OpExtension "SPV_GOOGLE_hlsl_functionality1" // Input control point diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.ps.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.ps.hlsl index 90a842b2f..e8076e646 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.interface.ps.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.interface.ps.hlsl @@ -3,7 +3,6 @@ // CHECK: OpCapability ClipDistance // CHECK: OpCapability CullDistance -// CHECK: OpExtension "SPV_GOOGLE_decorate_string" // CHECK: OpExtension "SPV_GOOGLE_hlsl_functionality1" struct Inner { diff --git a/tools/clang/test/CodeGenSPIRV/spirv.interface.vs.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.interface.vs.hlsl index 79c3b1284..5a3f7b211 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.interface.vs.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.interface.vs.hlsl @@ -3,7 +3,6 @@ // CHECK: OpCapability ClipDistance // CHECK: OpCapability CullDistance -// CHECK: OpExtension "SPV_GOOGLE_decorate_string" // CHECK: OpExtension "SPV_GOOGLE_hlsl_functionality1" // CHECK: OpEntryPoint Vertex %main "main" %gl_PerVertexOut %in_var_TEXCOORD %in_var_SV_Position %in_var_SV_ClipDistance %in_var_SV_CullDistance0 %out_var_COLOR %out_var_TEXCOORD From b6d65cbe272749467124d282a24b805855107dcb Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 16 Apr 2018 11:22:40 -0400 Subject: [PATCH 13/15] [spirv] Support 16-bit types in stage IO (#1227) This feature will require the SPV_KHR_16bit_storage extension. --- .../include/clang/SPIRV/FeatureManager.h | 2 +- tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 7 +++ tools/clang/lib/SPIRV/DeclResultIdMapper.h | 1 - tools/clang/lib/SPIRV/FeatureManager.cpp | 22 ++++---- tools/clang/lib/SPIRV/TypeTranslator.cpp | 22 ++++++++ tools/clang/lib/SPIRV/TypeTranslator.h | 6 +-- .../CodeGenSPIRV/spirv.stage-io.16bit.hlsl | 53 +++++++++++++++++++ .../unittests/SPIRV/CodeGenSPIRVTest.cpp | 4 ++ 8 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/spirv.stage-io.16bit.hlsl diff --git a/tools/clang/include/clang/SPIRV/FeatureManager.h b/tools/clang/include/clang/SPIRV/FeatureManager.h index 9c4567933..d476e9a30 100644 --- a/tools/clang/include/clang/SPIRV/FeatureManager.h +++ b/tools/clang/include/clang/SPIRV/FeatureManager.h @@ -15,7 +15,6 @@ #include - #include "spirv-tools/libspirv.h" #include "clang/Basic/Diagnostic.h" @@ -31,6 +30,7 @@ namespace spirv { /// A list of SPIR-V extensions known to our CodeGen. enum class Extension { KHR = 0, + KHR_16bit_storage, KHR_device_group, KHR_multiview, KHR_shader_draw_parameters, diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 9a1d222fe..82ee61f5e 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1322,6 +1322,13 @@ bool DeclResultIdMapper::createStageVars(const hlsl::SigPoint *sigPoint, // Mark that we have used one index for this semantic ++semanticToUse->index; + // Require extension and capability if using 16-bit types + if (typeTranslator.getElementSpirvBitwidth(type) == 16) { + theBuilder.addExtension(Extension::KHR_16bit_storage, + "16-bit stage IO variables", decl->getLocation()); + theBuilder.requireCapability(spv::Capability::StorageInputOutput16); + } + // TODO: the following may not be correct? if (sigPoint->GetSignatureKind() == hlsl::DXIL::SignatureKind::PatchConstant) diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.h b/tools/clang/lib/SPIRV/DeclResultIdMapper.h index 733ad5767..a469c2853 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.h +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.h @@ -39,7 +39,6 @@ public: inline StageVar(const hlsl::SigPoint *sig, llvm::StringRef semaStr, const hlsl::Semantic *sema, llvm::StringRef semaName, uint32_t semaIndex, const VKBuiltInAttr *builtin, - uint32_t type, uint32_t locCount) : sigPoint(sig), semanticStr(semaStr), semantic(sema), semanticName(semaName), semanticIndex(semaIndex), builtinAttr(builtin), diff --git a/tools/clang/lib/SPIRV/FeatureManager.cpp b/tools/clang/lib/SPIRV/FeatureManager.cpp index ef03e6b31..a1651aaa5 100644 --- a/tools/clang/lib/SPIRV/FeatureManager.cpp +++ b/tools/clang/lib/SPIRV/FeatureManager.cpp @@ -84,7 +84,8 @@ bool FeatureManager::requestTargetEnv(spv_target_env requestedEnv, if (targetEnv == SPV_ENV_VULKAN_1_0 && requestedEnv == SPV_ENV_VULKAN_1_1) { emitError("Vulkan 1.1 is required for %0 but not permitted to use", srcLoc) << target; - emitNote("please specify your target environment via command line option -fspv-target-env=", + emitNote("please specify your target environment via command line option " + "-fspv-target-env=", {}); return false; } @@ -94,6 +95,7 @@ bool FeatureManager::requestTargetEnv(spv_target_env requestedEnv, Extension FeatureManager::getExtensionSymbol(llvm::StringRef name) { return llvm::StringSwitch(name) .Case("KHR", Extension::KHR) + .Case("SPV_KHR_16bit_storage", Extension::KHR_16bit_storage) .Case("SPV_KHR_device_group", Extension::KHR_device_group) .Case("SPV_KHR_multiview", Extension::KHR_multiview) .Case("SPV_KHR_shader_draw_parameters", @@ -115,6 +117,8 @@ const char *FeatureManager::getExtensionName(Extension symbol) { switch (symbol) { case Extension::KHR: return "KHR"; + case Extension::KHR_16bit_storage: + return "SPV_KHR_16bit_storage"; case Extension::KHR_device_group: return "SPV_KHR_device_group"; case Extension::KHR_multiview: @@ -164,19 +168,15 @@ bool FeatureManager::isExtensionRequiredForTargetEnv(Extension ext) { bool required = true; if (targetEnv == SPV_ENV_VULKAN_1_1) { // The following extensions are incorporated into Vulkan 1.1, and are - // therefore not required to be emitted for that target environment. The - // last 3 are currently not supported by the FeatureManager. - // TODO: Add the last 3 extensions to the list if we start to support them. - // SPV_KHR_shader_draw_parameters - // SPV_KHR_device_group - // SPV_KHR_multiview - // SPV_KHR_16bit_storage - // SPV_KHR_storage_buffer_storage_class - // SPV_KHR_variable_pointers + // therefore not required to be emitted for that target environment. + // TODO: Also add the following extensions if we start to support them. + // * SPV_KHR_storage_buffer_storage_class + // * SPV_KHR_variable_pointers switch (ext) { - case Extension::KHR_shader_draw_parameters: + case Extension::KHR_16bit_storage: case Extension::KHR_device_group: case Extension::KHR_multiview: + case Extension::KHR_shader_draw_parameters: required = false; } } diff --git a/tools/clang/lib/SPIRV/TypeTranslator.cpp b/tools/clang/lib/SPIRV/TypeTranslator.cpp index f97a29080..35397f955 100644 --- a/tools/clang/lib/SPIRV/TypeTranslator.cpp +++ b/tools/clang/lib/SPIRV/TypeTranslator.cpp @@ -395,12 +395,34 @@ uint32_t TypeTranslator::getElementSpirvBitwidth(QualType type) { return getElementSpirvBitwidth(elemType); } + // Matrix types + if (hlsl::IsHLSLMatType(type)) + return getElementSpirvBitwidth(hlsl::GetHLSLMatElementType(type)); + + // Array types + if (const auto *arrayType = type->getAsArrayTypeUnsafe()) { + return getElementSpirvBitwidth(arrayType->getElementType()); + } + + // Typedefs + if (const auto *typedefType = type->getAs()) + return getLocationCount(typedefType->desugar()); + + // Reference types + if (const auto *refType = type->getAs()) + return getLocationCount(refType->getPointeeType()); + + // Pointer types + if (const auto *ptrType = type->getAs()) + return getLocationCount(ptrType->getPointeeType()); + // Scalar types QualType ty = {}; const bool isScalar = isScalarType(type, &ty); assert(isScalar); if (const auto *builtinType = ty->getAs()) { switch (builtinType->getKind()) { + case BuiltinType::Bool: case BuiltinType::Int: case BuiltinType::UInt: case BuiltinType::Float: diff --git a/tools/clang/lib/SPIRV/TypeTranslator.h b/tools/clang/lib/SPIRV/TypeTranslator.h index 9f81a1069..53d84f742 100644 --- a/tools/clang/lib/SPIRV/TypeTranslator.h +++ b/tools/clang/lib/SPIRV/TypeTranslator.h @@ -140,9 +140,9 @@ public: uint32_t getTypeWithCustomBitwidth(QualType type, uint32_t bitwidth); /// \brief Returns the realized bitwidth of the given type when represented in - /// SPIR-V. Panics if the given type is not a scalar or vector of float or - /// integer. In case of vectors, it returns the realized SPIR-V bitwidth of - /// the vector elements. + /// SPIR-V. Panics if the given type is not a scalar, a vector/matrix of float + /// or integer, or an array of them. In case of vectors, it returns the + /// realized SPIR-V bitwidth of the vector elements. uint32_t getElementSpirvBitwidth(QualType type); /// \brief Returns true if the given type will be translated into a SPIR-V diff --git a/tools/clang/test/CodeGenSPIRV/spirv.stage-io.16bit.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.stage-io.16bit.hlsl new file mode 100644 index 000000000..33ce60a7b --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/spirv.stage-io.16bit.hlsl @@ -0,0 +1,53 @@ +// Run: %dxc -T vs_6_2 -E main -enable-16bit-types + +// CHECK: OpCapability StorageInputOutput16 + +// CHECK: OpExtension "SPV_KHR_16bit_storage" + +// CHECK: OpDecorate %in_var_A Location 0 +// CHECK: OpDecorate %in_var_B Location 4 +// CHECK: OpDecorate %in_var_C Location 6 +// CHECK: OpDecorate %in_var_D Location 7 +// CHECK: OpDecorate %in_var_E Location 8 + +// CHECK: OpDecorate %out_var_A Location 0 +// CHECK: OpDecorate %out_var_B Location 2 +// CHECK: OpDecorate %out_var_C Location 6 +// CHECK: OpDecorate %out_var_D Location 7 +// CHECK: OpDecorate %out_var_E Location 8 + +// CHECK: %in_var_A = OpVariable %_ptr_Input__arr_v2half_uint_4 Input +// CHECK: %in_var_B = OpVariable %_ptr_Input__arr_v3ushort_uint_2 Input +// CHECK: %in_var_C = OpVariable %_ptr_Input_short Input +// CHECK: %in_var_D = OpVariable %_ptr_Input_v2ushort Input +// CHECK: %in_var_E = OpVariable %_ptr_Input_mat3v2half Input + +// CHECK: %out_var_A = OpVariable %_ptr_Output_mat2v3half Output +// CHECK: %out_var_B = OpVariable %_ptr_Output__arr_v2short_uint_4 Output +// CHECK: %out_var_C = OpVariable %_ptr_Output_half Output +// CHECK: %out_var_D = OpVariable %_ptr_Output_v2short Output +// CHECK: %out_var_E = OpVariable %_ptr_Output_v3ushort Output + +struct VSOut { + half2x3 outA : A; // 2 locations: 0, 1 + int16_t2 outB[4] : B; // 4 locations: 2, 3, 4, 5 + half outC : C; // 1 location : 6 + int16_t2 outD : D; // 1 location : 7 + uint16_t3 outE : E; // 1 location : 8 +}; + +VSOut main( + half2 inA[4] : A, // 4 locations: 0, 1, 2, 3 + uint16_t2x3 inB : B, // 2 locations: 4, 5 + int16_t inC : C, // 1 location : 6 + uint16_t2 inD : D, // 1 location : 7 + float16_t3x2 inE : E // 3 location : 8, 9, 10 +) { + VSOut o; + o.outA = inA[0].x; + o.outB[0] = inB[0][0]; + o.outC = inC.x; + o.outD = inD[0]; + o.outE = inE[0][0]; + return o; +} diff --git a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp index 60765f26b..dabe0da91 100644 --- a/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp +++ b/tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp @@ -1216,6 +1216,10 @@ TEST_F(FileTest, SpirvStageIOInterfacePS) { runFileTest("spirv.interface.ps.hlsl"); } +TEST_F(FileTest, SpirvStageIO16bitTypes) { + runFileTest("spirv.stage-io.16bit.hlsl"); +} + TEST_F(FileTest, SpirvInterpolation) { runFileTest("spirv.interpolation.hlsl"); } From 4ac938cc5bcfe03297724b337914a864135bd3fb Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 16 Apr 2018 17:16:07 -0400 Subject: [PATCH 14/15] [spirv] Clarify doc about SPIR-V target environment (#1230) --- docs/SPIR-V.rst | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index ab42bb6a7..3660d2ff3 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -297,22 +297,24 @@ interface variables: SPIR-V version and extension ---------------------------- -In the **defult** mode (without ``-fspv-extension=`` command-line -option), SPIR-V CodeGen will try its best to use the lowest SPIR-V version, and -only require higher SPIR-V versions and extensions when they are truly needed -for translating the input source code. +SPIR-V CodeGen provides two command-line options for fine-grained SPIR-V target +environment (hence SPIR-V version) and SPIR-V extension control: -For example, unless `Shader Model 6.0 wave intrinsics`_ are used, the generated -SPIR-V will always be of version 1.0. The ``SPV_KHR_multivew`` extension will -not be emitted unless you use ``SV_ViewID``. +- ``-fspv-target-env=``: for specifying SPIR-V target environment +- ``-fspv-extension=``: for specifying allowed SPIR-V extensions -You can of course have fine-grained control of what extensions are permitted -in the CodeGen using the **explicit** mode, turned on by the -``-fspv-extension=`` command-line option. Only extensions supplied -via ``-fspv-extension=`` will be used. If that does not suffice, errors will -be emitted explaining what additional extensions are required to translate what -specific feature in the source code. If you want to allow all KHR extensions, -you can use ``-fspv-extension=KHR``. +``-fspv-target-env=`` only accepts ``vulkan1.0`` and ``vulkan1.1`` right now. +If such an option is not given, the CodeGen defaults to ``vulkan1.0``. When +targeting ``vulkan1.0``, trying to use features that are only available +in Vulkan 1.1 (SPIR-V 1.3), like `Shader Model 6.0 wave intrinsics`_, will +trigger a compiler error. + +If ``-fspv-extension=`` is not specified, the CodeGen will select suitable +SPIR-V extensions to translate the source code. Otherwise, only extensions +supplied via ``-fspv-extension=`` will be used. If that does not suffice, errors +will be emitted explaining what additional extensions are required to translate +what specific feature in the source code. If you want to allow all KHR +extensions, you can use ``-fspv-extension=KHR``. Legalization, optimization, validation -------------------------------------- @@ -667,12 +669,12 @@ right now: Uniform Buffer Layout" and "Standard Storage Buffer Layout" `_, respectively. They are the default. -2. Strict OpenGL ``std140`` for uniform buffers and strict OpenGL ``std430`` - for storage buffers: they allow packing data on the application side that - can be shared with OpenGL. They can be enabled by ``-fvk-use-gl-layout``. -3. DirectX memory layout rules for uniform buffers and storage buffers: +2. DirectX memory layout rules for uniform buffers and storage buffers: they allow packing data on the application side that can be shared with DirectX. They can be enabled by ``-fvk-use-dx-layout``. +3. Strict OpenGL ``std140`` for uniform buffers and strict OpenGL ``std430`` + for storage buffers: they allow packing data on the application side that + can be shared with OpenGL. They can be enabled by ``-fvk-use-gl-layout``. In the above, "vector-relaxed OpenGL ``std140``/``std430``" rules mean OpenGL ``std140``/``std430`` rules with the following modification for vector type @@ -2631,11 +2633,10 @@ generated. ``.RestartStrip()`` method calls will be translated into the SPIR-V Shader Model 6.0 Wave Intrinsics ================================ -:: - Wave intrinsics requires SPIR-V 1.3, which is supported by Vulkan 1.1. - If you use wave intrinsics in your source code, the generated SPIR-V code - will be of version 1.3 instead of 1.0, which is supported by Vulkan 1.0. +Note that Wave intrinsics requires SPIR-V 1.3, which is supported by Vulkan 1.1. +If you use wave intrinsics in your source code, you will need to specify +-fspv-target-env=vulkan1.1 via the command line to target Vulkan 1.1. Shader model 6.0 introduces a set of wave operations. Apart from ``WaveGetLaneCount()`` and ``WaveGetLaneIndex()``, which are translated into From 4491a30fba528b74c64e8f1f5b23a970de041f61 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Mon, 16 Apr 2018 17:21:02 -0400 Subject: [PATCH 15/15] [spirv] Support non-constant offsets in .Gather*() methods (#1232) They are emulated via 4 separate OpImage*Gather instructions. --- docs/SPIR-V.rst | 4 +- tools/clang/lib/SPIRV/SPIRVEmitter.cpp | 42 ++++++++++++++----- .../texture.array.gather-red.hlsl | 15 ++++++- .../test/CodeGenSPIRV/texture.gather-red.hlsl | 15 ++++++- 4 files changed, 62 insertions(+), 14 deletions(-) diff --git a/docs/SPIR-V.rst b/docs/SPIR-V.rst index 3660d2ff3..7cf64ec31 100644 --- a/docs/SPIR-V.rst +++ b/docs/SPIR-V.rst @@ -2199,7 +2199,9 @@ There are a few overloads for these functions: - For those overloads taking 4 offset parameters, those offset parameters will be conveyed as an additional ``ConstOffsets`` image operands to the - instruction. So those offset parameters must all be constant values. + instruction if those offset parameters are all constants. Otherwise, + 4 separate ``OpImageGather`` instructions will be emitted to get each texel + from each offset, using the ``Offset`` image operands. - For those overloads with the ``status`` parameter, ``OpImageSparseGather`` is used instead, and the resulting SPIR-V ``Residency Code`` will be written to ``status``. diff --git a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp index 68b4c02a1..59d7d8f44 100644 --- a/tools/clang/lib/SPIRV/SPIRVEmitter.cpp +++ b/tools/clang/lib/SPIRV/SPIRVEmitter.cpp @@ -2939,6 +2939,7 @@ uint32_t SPIRVEmitter::processTextureGatherRGBACmpRGBA( const uint32_t compareVal = isCmp ? doExpr(expr->getArg(2)) : 0; // Handle offsets (if any). + bool needsEmulation = false; uint32_t constOffset = 0, varOffset = 0, constOffsets = 0; if (numOffsetArgs == 1) { // The offset arg is not optional. @@ -2949,21 +2950,40 @@ uint32_t SPIRVEmitter::processTextureGatherRGBACmpRGBA( const auto offset2 = tryToEvaluateAsConst(expr->getArg(4 + isCmp)); const auto offset3 = tryToEvaluateAsConst(expr->getArg(5 + isCmp)); - // Make sure we can generate the ConstOffsets image operands in SPIR-V. - if (!offset0 || !offset1 || !offset2 || !offset3) { - emitError("all offset parameters to '%0' method call must be constants", - expr->getExprLoc()) - << callee->getName() << expr->getSourceRange(); - return 0; + // If any of the offsets is not constant, we then need to emulate the call + // using 4 OpImageGather instructions. Otherwise, we can leverage the + // ConstOffsets image operand. + if (offset0 && offset1 && offset2 && offset3) { + const uint32_t v2i32 = + theBuilder.getVecType(theBuilder.getInt32Type(), 2); + const uint32_t offsetType = + theBuilder.getArrayType(v2i32, theBuilder.getConstantUint32(4)); + constOffsets = theBuilder.getConstantComposite( + offsetType, {offset0, offset1, offset2, offset3}); + } else { + needsEmulation = true; } - const uint32_t v2i32 = theBuilder.getVecType(theBuilder.getInt32Type(), 2); - const uint32_t offsetType = - theBuilder.getArrayType(v2i32, theBuilder.getConstantUint32(4)); - constOffsets = theBuilder.getConstantComposite( - offsetType, {offset0, offset1, offset2, offset3}); } const auto status = hasStatusArg ? doExpr(expr->getArg(numArgs - 1)) : 0; + + if (needsEmulation) { + const auto elemType = typeTranslator.translateType( + hlsl::GetHLSLVecElementType(callee->getReturnType())); + + uint32_t texels[4]; + for (uint32_t i = 0; i < 4; ++i) { + varOffset = doExpr(expr->getArg(2 + isCmp + i)); + const uint32_t gatherRet = theBuilder.createImageGather( + retTypeId, imageTypeId, image, sampler, coordinate, + theBuilder.getConstantInt32(component), compareVal, /*constOffset*/ 0, + varOffset, /*constOffsets*/ 0, /*sampleNumber*/ 0, status); + texels[i] = theBuilder.createCompositeExtract(elemType, gatherRet, {i}); + } + return theBuilder.createCompositeConstruct( + retTypeId, {texels[0], texels[1], texels[2], texels[3]}); + } + return theBuilder.createImageGather( retTypeId, imageTypeId, image, sampler, coordinate, theBuilder.getConstantInt32(component), compareVal, constOffset, diff --git a/tools/clang/test/CodeGenSPIRV/texture.array.gather-red.hlsl b/tools/clang/test/CodeGenSPIRV/texture.array.gather-red.hlsl index 3adb9675a..d3915dc12 100644 --- a/tools/clang/test/CodeGenSPIRV/texture.array.gather-red.hlsl +++ b/tools/clang/test/CodeGenSPIRV/texture.array.gather-red.hlsl @@ -21,7 +21,7 @@ TextureCubeArray tCubeArray : register(t3); // CHECK: %SparseResidencyStruct_0 = OpTypeStruct %uint %v4int -float4 main(float3 location: A) : SV_Target { +float4 main(float3 location: A, int2 offset : B) : SV_Target { // CHECK: [[t2f4:%\d+]] = OpLoad %type_2d_image_array %t2f4 // CHECK-NEXT: [[gSampler:%\d+]] = OpLoad %type_sampler %gSampler // CHECK-NEXT: [[loc:%\d+]] = OpLoad %v3float %location @@ -85,5 +85,18 @@ float4 main(float3 location: A) : SV_Target { // CHECK-NEXT: OpStore %g [[result]] int4 g = tCubeArray.GatherRed(gSampler, /*location*/ float4(1.5, 1.5, 1.5, 1.5), status); +// CHECK: [[offset:%\d+]] = OpLoad %v2int %offset +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[offset]] +// CHECK: [[texel0:%\d+]] = OpCompositeExtract %float [[gather]] 0 +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[c34]] +// CHECK: [[texel1:%\d+]] = OpCompositeExtract %float [[gather]] 1 +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[c56]] +// CHECK: [[texel2:%\d+]] = OpCompositeExtract %float [[gather]] 2 +// CHECK: [[offset:%\d+]] = OpLoad %v2int %offset +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[offset]] +// CHECK: [[texel3:%\d+]] = OpCompositeExtract %float [[gather]] 3 +// CHECK: OpCompositeConstruct %v4float [[texel0]] [[texel1]] [[texel2]] [[texel3]] + float4 h = t2f4.GatherRed(gSampler, location, offset, int2(3, 4), int2(5, 6), offset); + return 1.0; } diff --git a/tools/clang/test/CodeGenSPIRV/texture.gather-red.hlsl b/tools/clang/test/CodeGenSPIRV/texture.gather-red.hlsl index 35137af69..2f6d22c8d 100644 --- a/tools/clang/test/CodeGenSPIRV/texture.gather-red.hlsl +++ b/tools/clang/test/CodeGenSPIRV/texture.gather-red.hlsl @@ -22,7 +22,7 @@ TextureCube tCube : register(t3); // CHECK: %SparseResidencyStruct_0 = OpTypeStruct %uint %v4int -float4 main(float2 location: A) : SV_Target { +float4 main(float2 location: A, int2 offset : B) : SV_Target { // CHECK: [[t2f4:%\d+]] = OpLoad %type_2d_image %t2f4 // CHECK-NEXT: [[gSampler:%\d+]] = OpLoad %type_sampler %gSampler // CHECK-NEXT: [[loc:%\d+]] = OpLoad %v2float %location @@ -86,5 +86,18 @@ float4 main(float2 location: A) : SV_Target { // CHECK-NEXT: OpStore %g [[result]] int4 g = tCube.GatherRed(gSampler, /*location*/ float3(1.5, 1.5, 1.5), status); +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[c12]] +// CHECK: [[texel0:%\d+]] = OpCompositeExtract %float [[gather]] 0 +// CHECK: [[offset:%\d+]] = OpLoad %v2int %offset +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[offset]] +// CHECK: [[texel1:%\d+]] = OpCompositeExtract %float [[gather]] 1 +// CHECK: [[offset:%\d+]] = OpLoad %v2int %offset +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[offset]] +// CHECK: [[texel2:%\d+]] = OpCompositeExtract %float [[gather]] 2 +// CHECK: [[gather:%\d+]] = OpImageGather %v4float {{%\d+}} {{%\d+}} %int_0 Offset [[c78]] +// CHECK: [[texel3:%\d+]] = OpCompositeExtract %float [[gather]] 3 +// CHECK: OpCompositeConstruct %v4float [[texel0]] [[texel1]] [[texel2]] [[texel3]] + float4 h = t2f4.GatherRed(gSampler, location, int2(1, 2), offset, offset, int2(7, 8)); + return 1.0; }