diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index 19c0d75174..75eba81c57 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -30,6 +30,9 @@ line upon naming the release. Refer to previous for appropriate section names. - SPIR-V: Fixed an invalid `OpSelect` being generated when optimizing for SPIR-V 1.3 and earlier [#8603](https://github.com/microsoft/DirectXShaderCompiler/issues/8603). +- Fix a crash generating DXIL from sources containing a dynamic resource heap + access that was discarded. Identified during development of SPIR-V support for + [descriptor heaps](https://github.com/microsoft/DirectXShaderCompiler/pull/8517#discussion_r3752113078). #### HLSL Language diff --git a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp index 13edadf9df..7aa5eb4ad4 100644 --- a/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp +++ b/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp @@ -12,6 +12,7 @@ #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Analysis/DxilValueCache.h" +#include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/CFG.h" #include "llvm/IR/DerivedTypes.h" #include "llvm/IR/Function.h" @@ -330,6 +331,13 @@ void LowerGetResourceFromHeap( for (auto uit = resPtr->user_begin(); uit != resPtr->user_end();) { User *U = *(uit++); BitCastInst *BCI = cast(U); + // Ignore uses of the resource which are just bitcasts to i8* for lifetime + // markers. These will get cleaned up in later legalization. + if (BCI->getType()->getPointerElementType()->isIntegerTy(8)) { + DXASSERT(onlyUsedByLifetimeMarkers(BCI), + "otherwise, unexpected use of i8* cast of resource ptr"); + continue; + } DXASSERT( dxilutil::IsHLSLResourceType( BCI->getType()->getPointerElementType()) || @@ -348,7 +356,10 @@ void LowerGetResourceFromHeap( } BCI->eraseFromParent(); } - resPtr->eraseFromParent(); + // Only erase the resource if it has no remaining uses. The correct fix here + // is to just not generate these resources, but that is a larger change. + if (resPtr->use_empty()) + resPtr->eraseFromParent(); } } diff --git a/tools/clang/test/CodeGenDXIL/discarded_dynamic_res.hlsl b/tools/clang/test/CodeGenDXIL/discarded_dynamic_res.hlsl new file mode 100644 index 0000000000..b632762130 --- /dev/null +++ b/tools/clang/test/CodeGenDXIL/discarded_dynamic_res.hlsl @@ -0,0 +1,48 @@ +// RUN: %dxc -T cs_6_6 -fcgl %s | FileCheck %s +// RUN: %dxc -T cs_6_6 -Od -fcgl %s | FileCheck --check-prefix=NO_LIFETIMES %s + +// Regression test: indexing into a resource heap (or sampler heap) as a +// discarded-value expression (i.e. its result is never read) used to crash +// the compiler. Clang still emits an alloca plus lifetime markers for the +// implicit temporary in that case, and CGHLSLMSHelper::LowerGetResourceFromHeap +// only expected the resource pointer to be used through a resource-typed +// bitcast that is loaded from; it did not expect the i8* bitcast used by +// llvm.lifetime.start/llvm.lifetime.end, which triggered an invalid cast<> +// (see FinishIntrinsics -> LowerGetResourceFromHeap). + +// When `-Od` is passed lifetime markers are implicitly disabled, so the +// remainder of the IR changes are irrelevant. +// NO_LIFETIMES: define void @main + +// CHECK: [[ResourceX:%.*]] = alloca %struct..Resource +// CHECK: [[SamplerX:%.*]] = alloca %struct..Sampler +// CHECK: [[ResourceY:%.*]] = alloca %struct..Resource +// CHECK: [[SamplerY:%.*]] = alloca %struct..Sampler + +[numthreads(1, 1, 1)] +void main(uint3 tid : SV_DispatchThreadID) { + // CHECK: [[Start:%.*]] = bitcast %struct..Resource* [[ResourceX]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.start(i64 4, i8* [[Start]]) + // CHECK: [[End:%.*]] = bitcast %struct..Resource* [[ResourceX]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.end(i64 4, i8* [[End]]) + ResourceDescriptorHeap[tid.x]; + + // CHECK: [[Start:%.*]] = bitcast %struct..Sampler* [[SamplerX]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.start(i64 4, i8* [[Start]]) + // CHECK: [[End:%.*]] = bitcast %struct..Sampler* [[SamplerX]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.end(i64 4, i8* [[End]]) + SamplerDescriptorHeap[tid.x]; + + + // CHECK: [[Start:%.*]] = bitcast %struct..Resource* [[ResourceY]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.start(i64 4, i8* [[Start]]) + // CHECK: [[End:%.*]] = bitcast %struct..Resource* [[ResourceY]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.end(i64 4, i8* [[End]]) + (void)ResourceDescriptorHeap[tid.y]; + + // CHECK: [[Start:%.*]] = bitcast %struct..Sampler* [[SamplerY]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.start(i64 4, i8* [[Start]]) + // CHECK: [[End:%.*]] = bitcast %struct..Sampler* [[SamplerY]] to i8* + // CHECK-NEXT: call void @llvm.lifetime.end(i64 4, i8* [[End]]) + (void)SamplerDescriptorHeap[tid.y]; +}