Skip to content

Commit 6001784

Browse files
gpuav: Fix Slang doing real pointer math (KhronosGroup#11503)
1 parent b3b08da commit 6001784

File tree

3 files changed

+149
-21
lines changed

3 files changed

+149
-21
lines changed

layers/gpuav/spirv/buffer_device_address_pass.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2024-2025 LunarG, Inc.
1+
/* Copyright (c) 2024-2026 LunarG, Inc.
22
*
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
@@ -231,8 +231,16 @@ bool BufferDeviceAddressPass::Instrument() {
231231
load_type_pointer->inst_.StorageClass() == spv::StorageClassPhysicalStorageBuffer) {
232232
const Type* struct_type = type_manager_.FindTypeById(load_type_pointer->inst_.Operand(1));
233233
if (struct_type && struct_type->spv_type_ == SpvType::kStruct) {
234-
const uint32_t struct_offset =
235-
FindOffsetInStruct(struct_type->Id(), nullptr, false, access_chain_insts);
234+
uint32_t root_struct_id = struct_type->Id();
235+
236+
// GLSL/HLSL will only ever a struct, but for Slang, we might have the first access be the pointer and
237+
// we actually need that outer struct, which "looks" an OpTypePointer with an ArrayStride attached to it
238+
const Instruction* last_access = access_chain_insts.back();
239+
if (!last_access->IsNonPtrAccessChain() && last_access->TypeId() == load_type_pointer->Id()) {
240+
root_struct_id = load_type_pointer->Id();
241+
}
242+
243+
const uint32_t struct_offset = FindOffsetInStruct(root_struct_id, nullptr, false, access_chain_insts);
236244
if (struct_offset == 0) {
237245
continue;
238246
}

layers/gpuav/spirv/pass.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2024-2025 LunarG, Inc.
1+
/* Copyright (c) 2024-2026 LunarG, Inc.
22
*
33
* Licensed under the Apache License, Version 2.0 (the "License");
44
* you may not use this file except in compliance with the License.
@@ -681,16 +681,21 @@ uint32_t Pass::FindOffsetInStruct(uint32_t struct_id, const CooperativeMatrixAcc
681681

682682
current_type_id = current_type->inst_.Operand(constant_value); // Get element type for next step
683683
} break;
684+
case SpvType::kPointer: {
685+
// So what this means is we have a pointer of structs, found in Slang doing something like
686+
//
687+
// struct PerFrame {};
688+
// PerFrame* bufPerFrame;
689+
//
690+
// And the first PtrAccessChain is going to try and index into.
691+
// There should be ArrayStride to know how large the Struct is
692+
const uint32_t array_stride = GetDecoration(current_type_id, spv::DecorationArrayStride)->Word(3);
693+
current_offset = constant_value * array_stride;
694+
695+
current_type_id = current_type->inst_.Word(3); // Get pointer type
696+
} break;
684697
default: {
685-
// A non-composite implies that this must be the end of the access-chain chain
686-
auto next_access_chain_iter = (access_chain_iter)+1;
687-
if (next_access_chain_iter != access_chain_insts.rend())
688-
{
689-
module_.InternalError(Name(), "FindOffsetInStruct has unexpected non-composite type");
690-
break;
691-
}
692-
const uint32_t type_size = FindTypeByteSize(current_type_id);
693-
current_offset = constant_value * type_size;
698+
module_.InternalError(Name(), "FindOffsetInStruct has unexpected non-composite type");
694699
} break;
695700
}
696701

tests/unit/gpu_av_buffer_device_address_positive.cpp

Lines changed: 123 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/*
2-
* Copyright (c) 2020-2025 The Khronos Group Inc.
3-
* Copyright (c) 2020-2025 Valve Corporation
4-
* Copyright (c) 2020-2025 LunarG, Inc.
5-
* Copyright (c) 2020-2025 Google, Inc.
2+
* Copyright (c) 2020-2026 The Khronos Group Inc.
3+
* Copyright (c) 2020-2026 Valve Corporation
4+
* Copyright (c) 2020-2026 LunarG, Inc.
5+
* Copyright (c) 2020-2026 Google, Inc.
66
*
77
* Licensed under the Apache License, Version 2.0 (the "License");
88
* you may not use this file except in compliance with the License.
@@ -1381,11 +1381,11 @@ TEST_F(PositiveGpuAVBufferDeviceAddress, MultipleAccessChains) {
13811381
m_default_queue->SubmitAndWait(m_command_buffer);
13821382
}
13831383

1384-
TEST_F(PositiveGpuAVBufferDeviceAddress, AccessChainStartIsStructEndIsNonComposite) {
1385-
TEST_DESCRIPTION("Slang will produce a chain of access chains that start with a base of a composite and end on a result type of a non-composite");
1386-
1384+
TEST_F(PositiveGpuAVBufferDeviceAddress, DescriptorStructPointerSlang) {
1385+
TEST_DESCRIPTION(
1386+
"Slang will produce a chain of access chains that start with a base of a composite and end on a result type of a "
1387+
"non-composite");
13871388
RETURN_IF_SKIP(CheckSlangSupport());
1388-
13891389
RETURN_IF_SKIP(InitGpuVUBufferDeviceAddress(false));
13901390

13911391
const char *slang_shader = R"slang(
@@ -1426,6 +1426,121 @@ TEST_F(PositiveGpuAVBufferDeviceAddress, AccessChainStartIsStructEndIsNonComposi
14261426
m_default_queue->SubmitAndWait(m_command_buffer);
14271427
}
14281428

1429+
TEST_F(PositiveGpuAVBufferDeviceAddress, PushConstantStructPointerSlang) {
1430+
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/11348");
1431+
RETURN_IF_SKIP(CheckSlangSupport());
1432+
SetTargetApiVersion(VK_API_VERSION_1_2);
1433+
AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
1434+
AddRequiredFeature(vkt::Feature::shaderDrawParameters);
1435+
AddRequiredFeature(vkt::Feature::multiview);
1436+
RETURN_IF_SKIP(InitGpuVUBufferDeviceAddress(false));
1437+
InitRenderTarget();
1438+
1439+
const char *vert_source = R"(
1440+
float4 operator*(float4x4 a, float4 b) { return mul(b, a); }
1441+
1442+
struct PerFrame {
1443+
uint a;
1444+
uint b;
1445+
uint c;
1446+
uint d;
1447+
float4x4 proj[2];
1448+
};
1449+
1450+
struct PushConstants {
1451+
PerFrame* bufPerFrame;
1452+
};
1453+
1454+
[[vk::push_constant]] PushConstants pc;
1455+
1456+
struct VSOutput {
1457+
float4 position : SV_Position;
1458+
};
1459+
1460+
[shader("vertex")]
1461+
VSOutput main() {
1462+
PerFrame* perFrame = pc.bufPerFrame;
1463+
1464+
VSOutput out;
1465+
out.position = perFrame[2].proj[1] * float4(50.0);
1466+
1467+
return out;
1468+
}
1469+
)";
1470+
1471+
VkShaderObj vs(*m_device, vert_source, VK_SHADER_STAGE_VERTEX_BIT, SPV_ENV_VULKAN_1_2, SPV_SOURCE_SLANG);
1472+
1473+
VkPushConstantRange pc_range = {VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT, 0, 64};
1474+
const vkt::PipelineLayout pipeline_layout(*m_device, {}, {pc_range});
1475+
1476+
CreatePipelineHelper pipe(*this);
1477+
pipe.gp_ci_.layout = pipeline_layout;
1478+
pipe.shader_stages_ = {vs.GetStageCreateInfo(), pipe.fs_->GetStageCreateInfo()};
1479+
pipe.CreateGraphicsPipeline();
1480+
}
1481+
1482+
TEST_F(PositiveGpuAVBufferDeviceAddress, MultipleStructPointerSlang) {
1483+
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/11348");
1484+
RETURN_IF_SKIP(CheckSlangSupport());
1485+
SetTargetApiVersion(VK_API_VERSION_1_2);
1486+
AddRequiredFeature(vkt::Feature::bufferDeviceAddress);
1487+
AddRequiredFeature(vkt::Feature::shaderDrawParameters);
1488+
AddRequiredFeature(vkt::Feature::multiview);
1489+
RETURN_IF_SKIP(InitGpuVUBufferDeviceAddress(false));
1490+
InitRenderTarget();
1491+
1492+
const char *vert_source = R"(
1493+
struct Data {
1494+
float4 a;
1495+
float4 b;
1496+
float4 c;
1497+
float4 d;
1498+
}
1499+
1500+
struct PerFrame {
1501+
Data proj[4];
1502+
};
1503+
1504+
struct PushConstants {
1505+
// Each |PerFrame| is has a 256 array stride
1506+
PerFrame* bufPerFrame;
1507+
};
1508+
1509+
[[vk::push_constant]] PushConstants pc;
1510+
1511+
struct VSOutput {
1512+
float4 position : SV_Position;
1513+
};
1514+
1515+
[shader("vertex")]
1516+
VSOutput main() {
1517+
PerFrame* perFrame = pc.bufPerFrame;
1518+
1519+
VSOutput out;
1520+
// Range is from 0 to 1024
1521+
out.position = (
1522+
perFrame[0].proj[0].a *
1523+
perFrame[1].proj[1].b *
1524+
perFrame[2].proj[2].c *
1525+
perFrame[3].proj[3].d
1526+
)
1527+
* float4(50.0);
1528+
1529+
return out;
1530+
}
1531+
)";
1532+
1533+
VkShaderObj vs(*m_device, vert_source, VK_SHADER_STAGE_VERTEX_BIT, SPV_ENV_VULKAN_1_2, SPV_SOURCE_SLANG);
1534+
1535+
VkPushConstantRange pc_range = {VK_SHADER_STAGE_VERTEX_BIT | VK_SHADER_STAGE_FRAGMENT_BIT, 0, 64};
1536+
const vkt::PipelineLayout pipeline_layout(*m_device, {}, {pc_range});
1537+
1538+
CreatePipelineHelper pipe(*this);
1539+
pipe.gp_ci_.layout = pipeline_layout;
1540+
pipe.shader_stages_ = {vs.GetStageCreateInfo(), pipe.fs_->GetStageCreateInfo()};
1541+
pipe.CreateGraphicsPipeline();
1542+
}
1543+
14291544
TEST_F(PositiveGpuAVBufferDeviceAddress, MultipleAccessChainsDescriptorBuffer) {
14301545
TEST_DESCRIPTION("Slang will produce a chain of OpAccessChains");
14311546

0 commit comments

Comments
 (0)