From 4e799936fb45b149d8cbf33805ad0be6456efb91 Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Mon, 16 Sep 2024 14:41:53 -0400 Subject: [PATCH] Add specific handling for inline spirv pointer types (#6873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add specific handling for inline spirv pointer types We currently blindly create a new type for all vk::SpirvType* types. This can cause problems when the type is suppose to match another type. In this case, we want a spirv pointer type to match the pointer type of the variable. The OpTypePointer for the variable is implicitly created when declaring the variable, but, if we want to explicitly declare the pointer as a SpirvType, we get two different OpTypePointer instructions in the module. So technically the types do not match. To fix this, I add special handling in the SPIR-V backend to be able to merge the implicit pointer type created and the SpirvType when they match. This implements the SPIR-V Pointers in [HLSL spec proposal 0021](https://github.com/microsoft/hlsl-specs/blob/main/proposals/0021-vk-coop-matrix.md#spir-v-pointers) --------- Co-authored-by: Nathan Gauër --- external/SPIRV-Headers | 2 +- external/SPIRV-Tools | 2 +- tools/clang/lib/Headers/hlsl/vk/spirv.h | 34 ++++++++++++++ tools/clang/lib/SPIRV/LowerTypeVisitor.cpp | 45 +++++++++++++++++++ tools/clang/lib/SPIRV/LowerTypeVisitor.h | 7 +++ tools/clang/test/CMakeLists.txt | 2 + .../CodeGenSPIRV/workgroupspirvpointer.hlsl | 26 +++++++++++ .../workgroupspirvpointer.varpointer.hlsl | 23 ++++++++++ tools/clang/test/lit.cfg | 1 + tools/clang/test/lit.site.cfg.in | 1 + 10 files changed, 141 insertions(+), 2 deletions(-) create mode 100644 tools/clang/lib/Headers/hlsl/vk/spirv.h create mode 100644 tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.varpointer.hlsl diff --git a/external/SPIRV-Headers b/external/SPIRV-Headers index efb6b4099..db5a00f8c 160000 --- a/external/SPIRV-Headers +++ b/external/SPIRV-Headers @@ -1 +1 @@ -Subproject commit efb6b4099ddb8fa60f62956dee592c4b94ec6a49 +Subproject commit db5a00f8cebe81146cafabf89019674a3c4bf03d diff --git a/external/SPIRV-Tools b/external/SPIRV-Tools index 07f49ce65..72c291332 160000 --- a/external/SPIRV-Tools +++ b/external/SPIRV-Tools @@ -1 +1 @@ -Subproject commit 07f49ce65d350ba585ac0696ad5c868163928789 +Subproject commit 72c291332a0558ab4121eff9db97e428b574b58b diff --git a/tools/clang/lib/Headers/hlsl/vk/spirv.h b/tools/clang/lib/Headers/hlsl/vk/spirv.h new file mode 100644 index 000000000..294af0307 --- /dev/null +++ b/tools/clang/lib/Headers/hlsl/vk/spirv.h @@ -0,0 +1,34 @@ +// Copyright (c) 2024 Google LLC +// +// This file is licensed under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef _HLSL_VK_SPIRV_H_ +#define _HLSL_VK_SPIRV_H_ + +namespace vk { + +enum StorageClass { + StorageClassWorkgroup = 4, +}; + +// An opaque type to represent a Spir-V pointer to the workgroup storage class. +// clang-format off +template +using WorkgroupSpirvPointer = const vk::SpirvOpaqueType< + /* OpTypePointer */ 32, + vk::Literal >, + PointeeType>; +// clang-format on + +// Returns an opaque Spir-V pointer to v. The memory object v's storage class +// modifier must be groupshared. If the incorrect storage class is used, then +// there will be a validation error, and it will not show the correct +template +[[vk::ext_instruction(/* OpCopyObject */ 83)]] WorkgroupSpirvPointer +GetGroupSharedAddress([[vk::ext_reference]] T v); + +} // namespace vk + +#endif // _HLSL_VK_SPIRV_H_ diff --git a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp index 874fcb7ae..b847e1a04 100644 --- a/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp +++ b/tools/clang/lib/SPIRV/LowerTypeVisitor.cpp @@ -732,6 +732,15 @@ const SpirvType *LowerTypeVisitor::lowerInlineSpirvType( auto args = specDecl->getTemplateArgs()[operandsIndex].getPackAsArray(); + if (operandsIndex == 1 && args.size() == 2 && + static_cast(opcode) == spv::Op::OpTypePointer) { + const SpirvType *result = + getSpirvPointerFromInlineSpirvType(args, rule, isRowMajor, srcLoc); + if (result) { + return result; + } + } + for (TemplateArgument arg : args) { switch (arg.getKind()) { case TemplateArgument::ArgKind::Type: { @@ -1363,5 +1372,41 @@ LowerTypeVisitor::populateLayoutInformation( return result; } +const SpirvType *LowerTypeVisitor::getSpirvPointerFromInlineSpirvType( + ArrayRef args, SpirvLayoutRule rule, + Optional isRowMajor, SourceLocation location) { + + assert(args.size() == 2 && "OpTypePointer requires exactly 2 arguments."); + QualType scLiteralType = args[0].getAsType(); + SpirvConstant *constant = nullptr; + if (!getVkIntegralConstantValue(scLiteralType, constant, location) || + !constant) { + return nullptr; + } + if (!constant->isLiteral()) + return nullptr; + + auto *intConstant = dyn_cast(constant); + if (!intConstant) { + return nullptr; + } + + visitInstruction(constant); + spv::StorageClass storageClass = + static_cast(intConstant->getValue().getLimitedValue()); + + QualType pointeeType; + if (args[1].getKind() == TemplateArgument::ArgKind::Type) { + pointeeType = args[1].getAsType(); + } else { + TemplateName templateName = args[1].getAsTemplate(); + pointeeType = createASTTypeFromTemplateName(templateName); + } + + const SpirvType *pointeeSpirvType = + lowerType(pointeeType, rule, isRowMajor, location); + return spvContext.getPointerType(pointeeSpirvType, storageClass); +} + } // namespace spirv } // namespace clang diff --git a/tools/clang/lib/SPIRV/LowerTypeVisitor.h b/tools/clang/lib/SPIRV/LowerTypeVisitor.h index 895e0e6cf..96235d150 100644 --- a/tools/clang/lib/SPIRV/LowerTypeVisitor.h +++ b/tools/clang/lib/SPIRV/LowerTypeVisitor.h @@ -124,6 +124,13 @@ private: SpirvLayoutRule rule, const uint32_t fieldIndex); + /// Get a lowered SpirvPointer from the args to a SpirvOpaqueType. + /// The pointer will use the given layout rule. `isRowMajor` is used to + /// lower the pointee type. + const SpirvType *getSpirvPointerFromInlineSpirvType( + ArrayRef args, SpirvLayoutRule rule, + Optional isRowMajor, SourceLocation location); + private: ASTContext &astContext; /// AST context SpirvContext &spvContext; /// SPIR-V context diff --git a/tools/clang/test/CMakeLists.txt b/tools/clang/test/CMakeLists.txt index 51edbd767..0e4733ebb 100644 --- a/tools/clang/test/CMakeLists.txt +++ b/tools/clang/test/CMakeLists.txt @@ -9,6 +9,8 @@ endif () string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR ${LLVM_RUNTIME_OUTPUT_INTDIR}) +set(HLSL_HEADERS_DIR ${LLVM_SOURCE_DIR}/tools/clang/lib/Headers/hlsl) # HLSL Change + configure_lit_site_cfg( ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg diff --git a/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.hlsl b/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.hlsl new file mode 100644 index 000000000..34d8aa214 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.hlsl @@ -0,0 +1,26 @@ +// RUN: dxc -fspv-target-env=vulkan1.3 -T cs_6_0 -E main -spirv -HV 2021 -I %hlsl_headers %s 2>&1 | FileCheck %s + +#include "vk/spirv.h" + +// CHECK-NOT: OpCapability VariablePointers + +RWStructuredBuffer data; + +groupshared int shared_data[64]; + +[[vk::ext_instruction(/* OpLoad */ 61)]] int +Load(vk::WorkgroupSpirvPointer p); + +int foo(vk::WorkgroupSpirvPointer param) { + return Load(param); +} + +[numthreads(64, 1, 1)] void main() { +// CHECK: [[ac:%[0-9]+]] = OpAccessChain %_ptr_Workgroup_int %shared_data %int_0 +// CHECK: [[ld:%[0-9]+]] = OpLoad %int [[ac]] +// CHECK: [[ac:%[0-9]+]] = OpAccessChain %_ptr_StorageBuffer_int %data %int_0 %uint_0 +// CHECK: OpStore [[ac]] [[ld]] + + vk::WorkgroupSpirvPointer p = vk::GetGroupSharedAddress(shared_data[0]); + data[0] = foo(p); +} diff --git a/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.varpointer.hlsl b/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.varpointer.hlsl new file mode 100644 index 000000000..30abf2ff3 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/workgroupspirvpointer.varpointer.hlsl @@ -0,0 +1,23 @@ +// RUN: dxc -fspv-target-env=vulkan1.3 -T cs_6_0 -E main -spirv -HV 2021 -I %hlsl_headers %s 2>&1 | FileCheck %s + +#include "vk/spirv.h" + +// CHECK: OpCapability VariablePointers + +RWStructuredBuffer data; + +groupshared int shared_data[64]; + +[[vk::ext_instruction(/* OpLoad */ 61)]] int +Load(vk::WorkgroupSpirvPointer p); + +[[noinline]] +int foo(vk::WorkgroupSpirvPointer param) { + return Load(param); +} + +[[vk::ext_capability(/* VariablePointersCapability */ 4442)]] +[numthreads(64, 1, 1)] void main() { + vk::WorkgroupSpirvPointer p = vk::GetGroupSharedAddress(shared_data[0]); + data[0] = foo(p); +} diff --git a/tools/clang/test/lit.cfg b/tools/clang/test/lit.cfg index f1757b00e..5fc5d4a27 100644 --- a/tools/clang/test/lit.cfg +++ b/tools/clang/test/lit.cfg @@ -210,6 +210,7 @@ if has_plugins and config.llvm_plugin_ext: config.substitutions.append( ('%llvmshlibdir', config.llvm_shlib_dir) ) config.substitutions.append( ('%pluginext', config.llvm_plugin_ext) ) +config.substitutions.append( ('%hlsl_headers', config.hlsl_headers_dir) ) #HLSL change if config.clang_examples: config.available_features.add('examples') diff --git a/tools/clang/test/lit.site.cfg.in b/tools/clang/test/lit.site.cfg.in index 6a6fe2bfb..207450add 100644 --- a/tools/clang/test/lit.site.cfg.in +++ b/tools/clang/test/lit.site.cfg.in @@ -21,6 +21,7 @@ config.enable_shared = @ENABLE_SHARED@ config.enable_backtrace = "@ENABLE_BACKTRACES@" config.host_arch = "@HOST_ARCH@" config.spirv = "@ENABLE_SPIRV_CODEGEN@" =="ON" +config.hlsl_headers_dir = "@HLSL_HEADERS_DIR@" # HLSL change # Support substitution of the tools and libs dirs with user parameters. This is # used when we can't determine the tool dir at configuration time.