-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[DirectX] Fix DXIL container generating invalid PSV0 part for unbounded resources #163287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-backend-directx Author: Helena Kotas (hekota) ChangesFixes #159679 Full diff: https://github.com/llvm/llvm-project/pull/163287.diff 2 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
index ca81d30473c03..8ace2d2777c74 100644
--- a/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
+++ b/llvm/lib/Target/DirectX/DXContainerGlobals.cpp
@@ -28,6 +28,7 @@
#include "llvm/Support/MD5.h"
#include "llvm/TargetParser/Triple.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
+#include <cstdint>
#include <optional>
using namespace llvm;
@@ -193,7 +194,12 @@ void DXContainerGlobals::addResourcesForPSV(Module &M, PSVRuntimeInfo &PSV) {
dxbc::PSV::v2::ResourceBindInfo BindInfo;
BindInfo.Type = Type;
BindInfo.LowerBound = Binding.LowerBound;
- BindInfo.UpperBound = Binding.LowerBound + Binding.Size - 1;
+ assert(Binding.Size == UINT32_MAX ||
+ (uint64_t)Binding.LowerBound + Binding.Size - 1 <= UINT32_MAX &&
+ "Resource range is too large");
+ BindInfo.UpperBound = (Binding.Size == UINT32_MAX)
+ ? UINT32_MAX
+ : Binding.LowerBound + Binding.Size - 1;
BindInfo.Space = Binding.Space;
BindInfo.Kind = static_cast<dxbc::PSV::ResourceKind>(Kind);
BindInfo.Flags = Flags;
diff --git a/llvm/test/CodeGen/DirectX/ContainerData/PSVResources.ll b/llvm/test/CodeGen/DirectX/ContainerData/PSVResources.ll
index bea03102e4ccf..70224fce55407 100644
--- a/llvm/test/CodeGen/DirectX/ContainerData/PSVResources.ll
+++ b/llvm/test/CodeGen/DirectX/ContainerData/PSVResources.ll
@@ -94,6 +94,18 @@ define void @main() #0 {
%uav2_2 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
@llvm.dx.resource.handlefrombinding.tdx.TypedBuffer_f32_1_0(
i32 4, i32 0, i32 10, i32 5, ptr null)
+
+ ; RWBuffer<float4> UnboundedArray[] : register(u10, space5)
+; CHECK: - Type: UAVTyped
+; CHECK: Space: 5
+; CHECK: LowerBound: 10
+; CHECK: UpperBound: 4294967295
+; CHECK: Kind: TypedBuffer
+; CHECK: Flags:
+; CHECK: UsedByAtomic64: false
+ ; RWBuffer<float4> Buf = BufferArray[100];
+ %uav3 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
+ @llvm.dx.resource.handlefrombinding(i32 5, i32 10, i32 -1, i32 100, ptr null)
ret void
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; is there an error test for the overflow case?
There is not, it's just an assert here. It should be validated by the frontend: #136809 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…ed resources (llvm#163287) When calculating the upper bound for resource binding to be stored in the PSV0 part of the DXIL container, the compiler needs to take into account that the resource range could be _unbounded_, which is indicated by the binding size being `UINT32_MAX`. Fixes [llvm#159679](llvm#159679)
When calculating the upper bound for resource binding to be stored in the PSV0 part of the DXIL container, the compiler needs to take into account that the resource range could be unbounded, which is indicated by the binding size being
UINT32_MAX
.Fixes #159679