Skip to content

Commit 37e8fae

Browse files
jeffnnjeffnn
authored
PIX:Descend Type Hierarchy for Alloca'd structs (#5028) (#5038)
In some cases structs like the RayDesc built-in type (one of the arguments to TraceRays) will be placed in an alloca. The PIX annotation code wasn't properly counting all the leaf-node values of aggregates within such structs when determining offsets for later members. The result of this was erroneous attribution of writes to those members during PIX shader debugging. * PIX:Descend Type Hierarchy for Alloca'd structs * Add test, vectors are scalar-only * New file-check for metadata nodes (cherry picked from commit f98f8d6)
1 parent 4fefba3 commit 37e8fae

3 files changed

Lines changed: 156 additions & 5 deletions

File tree

‎lib/DxilPIXPasses/DxilAnnotateWithVirtualRegister.cpp‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343

4444
uint32_t CountStructMembers(llvm::Type const *pType) {
4545
uint32_t Count = 0;
46-
47-
if (auto *ST = llvm::dyn_cast<llvm::StructType>(pType)) {
46+
if (auto *VT = llvm::dyn_cast<llvm::VectorType>(pType)) {
47+
// Vector types can only contain scalars:
48+
Count = VT->getVectorNumElements();
49+
} else if (auto *ST = llvm::dyn_cast<llvm::StructType>(pType)) {
4850
for (auto &El : ST->elements()) {
4951
Count += CountStructMembers(El);
5052
}
@@ -385,11 +387,26 @@ void DxilAnnotateWithVirtualRegister::AnnotateGeneric(llvm::Instruction *pI) {
385387
llvm::dyn_cast<llvm::ConstantInt>(GEP->getOperand(2));
386388
if (OffsetAsInt != nullptr)
387389
{
388-
std::uint32_t Offset = static_cast<std::uint32_t>(
390+
std::uint32_t OffsetInElementsFromStructureStart = static_cast<std::uint32_t>(
389391
OffsetAsInt->getValue().getLimitedValue());
390-
DXASSERT(Offset < regSize,
392+
DXASSERT(OffsetInElementsFromStructureStart < regSize,
391393
"Structure member offset out of expected range");
392-
PixDxilReg::AddMD(m_DM->GetCtx(), pI, baseStructRegNum + Offset);
394+
std::uint32_t OffsetInValuesFromStructureStart =
395+
OffsetInElementsFromStructureStart;
396+
if (auto *ST = llvm::dyn_cast<llvm::StructType>(GEP->getPointerOperandType()
397+
->getPointerElementType())) {
398+
DXASSERT(OffsetInElementsFromStructureStart < ST->getNumElements(),
399+
"Offset into struct is bigger than struct");
400+
OffsetInValuesFromStructureStart = 0;
401+
for (std::uint32_t Element = 0;
402+
Element < OffsetInElementsFromStructureStart; ++Element) {
403+
OffsetInValuesFromStructureStart +=
404+
CountStructMembers(ST->getElementType(Element));
405+
}
406+
}
407+
PixDxilReg::AddMD(m_DM->GetCtx(), pI,
408+
baseStructRegNum +
409+
OffsetInValuesFromStructureStart);
393410
}
394411
}
395412
}

‎tools/clang/test/HLSLFileCheck/pix/AnnotateVirtualRegs-Raygen.hlsl‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: %dxc -Od -T lib_6_6 %s | %opt -S -dxil-annotate-with-virtual-regs | FileCheck %s
2+
3+
4+
/* To run locally run:
5+
%dxc -Od -T lib_6_6 %s -Fc %t.ll
6+
%opt %t.ll -S -dxil-annotate-with-virtual-regs | FileCheck %s
7+
*/
8+
9+
RaytracingAccelerationStructure scene : register(t0);
10+
11+
struct RayPayload
12+
{
13+
int3 color;
14+
};
15+
16+
[shader("raygeneration")]
17+
void ENTRY()
18+
{
19+
RayDesc ray = {{0,0,0}, {0,0,1}, 0.05, 1000.0};
20+
RayPayload pld;
21+
TraceRay(scene, 0 /*rayFlags*/, 0xFF /*rayMask*/, 0 /*sbtRecordOffset*/, 1 /*sbtRecordStride*/, 0 /*missIndex*/, ray, pld);
22+
}
23+
24+
// CHECK: {{.*}} = alloca %struct.RayDesc, align 4, !pix-dxil-inst-num {{.*}}, !pix-alloca-reg [[RDAlloca:![0-9]+]]
25+
// CHECK: {{.*}} = alloca %struct.RayPayload, align 4, !pix-dxil-inst-num {{.*}}, !pix-alloca-reg [[RPAlloca:![0-9]+]]
26+
// CHECK: {{.*}} = getelementptr inbounds %struct.RayDesc, %struct.RayDesc* {{.*}}, i32 0, i32 0, !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[RDGEP:![0-9]+]]
27+
// CHECK: {{.*}} = load i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @dx.nothing.a, i32 0, i32 0), !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[NothGEP:![0-9]+]]
28+
// CHECK: {{.*}} = getelementptr inbounds %struct.RayDesc, %struct.RayDesc* {{.*}}, i32 0, i32 1, !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[RDGEP2:![0-9]+]]
29+
// CHECK: {{.*}} = load i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @dx.nothing.a, i32 0, i32 0), !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[NothGEP2:![0-9]+]]
30+
// CHECK: {{.*}} = getelementptr inbounds %struct.RayDesc, %struct.RayDesc* {{.*}}, i32 0, i32 2, !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[RDGEP3:![0-9]+]]
31+
// CHECK: {{.*}} = load i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @dx.nothing.a, i32 0, i32 0), !pix-dxil-inst-num {{.*}}, !pix-dxil-reg [[NothGEP3:![0-9]+]]
32+
33+
// CHECK-DAG: [[RDAlloca]] = !{i32 1, i32 0, i32 8}
34+
// CHECK-DAG: [[RPAlloca]] = !{i32 1, i32 8, i32 3}
35+
// CHECK-DAG: [[RDGEP]] = !{i32 0, i32 0}
36+
// CHECK-DAG: [[NothGEP]] = !{i32 0, i32 11}
37+
// CHECK-DAG: [[RDGEP2]] = !{i32 0, i32 3}
38+
// CHECK-DAG: [[NothGEP2]] = !{i32 0, i32 12}
39+
// CHECK-DAG: [[RDGEP3]] = !{i32 0, i32 4}
40+
// CHECK-DAG: [[NothGEP3]] = !{i32 0, i32 13}

‎tools/clang/unittests/HLSL/PixTest.cpp‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ class PixTest {
203203
TEST_METHOD(AddToASPayload)
204204

205205
TEST_METHOD(PixStructAnnotation_Lib_DualRaygen)
206+
TEST_METHOD(PixStructAnnotation_Lib_RaygenAllocaStructAlignment)
206207

207208
TEST_METHOD(PixStructAnnotation_Simple)
208209
TEST_METHOD(PixStructAnnotation_CopiedStruct)
@@ -2682,6 +2683,99 @@ void Raygen1()
26822683
}
26832684
}
26842685

2686+
TEST_F(PixTest, PixStructAnnotation_Lib_RaygenAllocaStructAlignment) {
2687+
if (m_ver.SkipDxilVersion(1, 5)) return;
2688+
2689+
const char* hlsl = R"(
2690+
2691+
RaytracingAccelerationStructure Scene : register(t0, space0);
2692+
RWTexture2D<float4> RenderTarget : register(u0);
2693+
2694+
struct SceneConstantBuffer
2695+
{
2696+
float4x4 projectionToWorld;
2697+
float4 cameraPosition;
2698+
float4 lightPosition;
2699+
float4 lightAmbientColor;
2700+
float4 lightDiffuseColor;
2701+
};
2702+
2703+
ConstantBuffer<SceneConstantBuffer> g_sceneCB : register(b0);
2704+
2705+
struct RayPayload
2706+
{
2707+
float4 color;
2708+
};
2709+
2710+
inline void GenerateCameraRay(uint2 index, out float3 origin, out float3 direction)
2711+
{
2712+
float2 xy = index + 0.5f; // center in the middle of the pixel.
2713+
float2 screenPos = xy;// / DispatchRaysDimensions().xy * 2.0 - 1.0;
2714+
2715+
// Invert Y for DirectX-style coordinates.
2716+
screenPos.y = -screenPos.y;
2717+
2718+
// Unproject the pixel coordinate into a ray.
2719+
float4 world = /*mul(*/float4(screenPos, 0, 1)/*, g_sceneCB.projectionToWorld)*/;
2720+
2721+
//world.xyz /= world.w;
2722+
origin = world.xyz; //g_sceneCB.cameraPosition.xyz;
2723+
direction = float3(1,0,0);//normalize(world.xyz - origin);
2724+
}
2725+
2726+
void RaygenCommon()
2727+
{
2728+
float3 rayDir;
2729+
float3 origin;
2730+
2731+
// Generate a ray for a camera pixel corresponding to an index from the dispatched 2D grid.
2732+
GenerateCameraRay(DispatchRaysIndex().xy, origin, rayDir);
2733+
2734+
// Trace the ray.
2735+
// Set the ray's extents.
2736+
RayDesc ray;
2737+
ray.Origin = origin;
2738+
ray.Direction = rayDir;
2739+
// Set TMin to a non-zero small value to avoid aliasing issues due to floating - point errors.
2740+
// TMin should be kept small to prevent missing geometry at close contact areas.
2741+
ray.TMin = 0.001;
2742+
ray.TMax = 10000.0;
2743+
RayPayload payload = { float4(0, 0, 0, 0) };
2744+
TraceRay(Scene, RAY_FLAG_CULL_BACK_FACING_TRIANGLES, ~0, 0, 1, 0, ray, payload);
2745+
2746+
// Write the raytraced color to the output texture.
2747+
// RenderTarget[DispatchRaysIndex().xy] = payload.color;
2748+
}
2749+
2750+
[shader("raygeneration")]
2751+
void Raygen()
2752+
{
2753+
RaygenCommon();
2754+
}
2755+
)";
2756+
2757+
auto Testables = TestStructAnnotationCase(hlsl, L"-Od", true, L"lib_6_6");
2758+
2759+
// Built-in type "RayDesc" has this structure: struct { float3 Origin; float
2760+
// TMin; float3 Direction; float TMax; } This is 8 floats, with members at
2761+
// offsets 0,3,4,7 respectively.
2762+
2763+
auto FindAtLeastOneOf = [=](char const *name, uint32_t index) {
2764+
VERIFY_IS_TRUE(std::find_if(Testables.AllocaWrites.begin(),
2765+
Testables.AllocaWrites.end(),
2766+
[&name, &index](AllocaWrite const &aw) {
2767+
return 0 == strcmp(aw.memberName.c_str(),
2768+
name) &&
2769+
aw.index == index;
2770+
}) != Testables.AllocaWrites.end());
2771+
};
2772+
2773+
FindAtLeastOneOf("Origin.x", 0);
2774+
FindAtLeastOneOf("TMin", 3);
2775+
FindAtLeastOneOf("Direction.x", 4);
2776+
FindAtLeastOneOf("TMax", 7);
2777+
}
2778+
26852779
TEST_F(PixTest, PixStructAnnotation_Simple) {
26862780
if (m_ver.SkipDxilVersion(1, 5))
26872781
return;

0 commit comments

Comments
 (0)