From e629824497887353a9886755c24e5ec7ba0f342c Mon Sep 17 00:00:00 2001 From: Steven Perron Date: Fri, 2 Aug 2024 11:30:33 -0400 Subject: [PATCH] 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. --- 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 + 8 files changed, 139 insertions(+) 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/tools/clang/lib/Headers/hlsl/vk/spirv.h b/tools/clang/lib/Headers/hlsl/vk/spirv.h new file mode 100644 index 0000000000..294af03078 --- /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 874fcb7aef..3b0ac7badc 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 && "OpTypePoint 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 895e0e6cfe..96235d1508 100644 --- a/tools/clang/lib/SPIRV/LowerTypeVisitor.h +++ b/tools/clang/lib/SPIRV/LowerTypeVisitor.h @@ -124,6 +124,13 @@ class LowerTypeVisitor : public Visitor { 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 ba3843f8d3..e77cffa473 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 0000000000..34d8aa214f --- /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 0000000000..30abf2ff31 --- /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 f1757b00e7..5fc5d4a27c 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 6a6fe2bfb0..207450add5 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.