Skip to content

Commit 43ffd43

Browse files
hekotahekota
committed
code review feedback - detect all collisions, change error message, add const ref
1 parent 6252a27 commit 43ffd43

File tree

6 files changed

+109
-21
lines changed

6 files changed

+109
-21
lines changed

‎llvm/lib/Target/DirectX/DXILPostOptimizationValidation.cpp‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ static void reportInvalidDirection(Module &M, DXILResourceMap &DRM) {
5353

5454
static void reportOverlappingError(Module &M, ResourceInfo R1,
5555
ResourceInfo R2) {
56-
SmallString<64> Message;
56+
SmallString<128> Message;
5757
raw_svector_ostream OS(Message);
5858
OS << "resource " << R1.getName() << " at register "
5959
<< R1.getBinding().LowerBound << " overlaps with resource " << R2.getName()
60-
<< " at register " << R2.getBinding().LowerBound << ", space "
60+
<< " at register " << R2.getBinding().LowerBound << " in space "
6161
<< R2.getBinding().Space;
6262
M.getContext().diagnose(DiagnosticInfoGeneric(Message));
6363
}
@@ -66,18 +66,20 @@ static void reportOverlappingBinding(Module &M, DXILResourceMap &DRM) {
6666
if (DRM.empty())
6767
return;
6868

69-
for (auto ResList :
69+
for (const auto &ResList :
7070
{DRM.srvs(), DRM.uavs(), DRM.cbuffers(), DRM.samplers()}) {
7171
if (ResList.empty())
7272
continue;
7373
const ResourceInfo *PrevRI = &*ResList.begin();
7474
for (auto *I = ResList.begin() + 1; I != ResList.end(); ++I) {
75-
const ResourceInfo *RI = &*I;
76-
if (PrevRI->getBinding().overlapsWith(RI->getBinding())) {
75+
const ResourceInfo *CurrentRI = &*I;
76+
const ResourceInfo *RI = CurrentRI;
77+
while (RI != ResList.end() &&
78+
PrevRI->getBinding().overlapsWith(RI->getBinding())) {
7779
reportOverlappingError(M, *PrevRI, *RI);
78-
continue;
80+
RI++;
7981
}
80-
PrevRI = RI;
82+
PrevRI = CurrentRI;
8183
}
8284
}
8385
}

‎llvm/test/CodeGen/DirectX/Binding/binding-overlap-1.ll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22

33
; Check overlap error for two resource arrays.
44

5-
; CHECK: error: resource A at register 0 overlaps with resource B at register 5, space 0
5+
; A overlaps with B
6+
; RWBuffer<float> A[10] : register(u0);
7+
; RWBuffer<float> B[10] : register(u5);
8+
9+
; CHECK: error: resource A at register 0 overlaps with resource B at register 5 in space 0
610

711
@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
812
@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
913

1014
define void @test_overlapping() {
1115
entry:
12-
; RWBuffer<float> A[10] : register(u0);
13-
; RWBuffer<float> B[10] : register(u5);
1416
%h1 = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 0, i32 10, i32 4, i1 false, ptr @A.str)
1517
%h2 = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 10, i32 4, i1 false, ptr @B.str)
1618
ret void

‎llvm/test/CodeGen/DirectX/Binding/binding-overlap-2.ll‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22

33
; Check overlap error for two resources with identical binding
44

5-
; CHECK: error: resource R at register 5 overlaps with resource S at register 5, space 10
5+
; R overlaps exactly with S
6+
; RWBuffer<float> R : register(u5, space10);
7+
; RWBuffer<float> S : register(u5, space10);
8+
9+
; CHECK: error: resource R at register 5 overlaps with resource S at register 5 in space 10
610

711
@R.str = private unnamed_addr constant [2 x i8] c"R\00", align 1
812
@S.str = private unnamed_addr constant [2 x i8] c"S\00", align 1
913

1014
define void @test_overlapping() {
1115
entry:
12-
; RWBuffer<float> R : register(u5, space10);
13-
; RWBuffer<float> S : register(u5, space10);
1416
%h1 = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 10, i32 5, i32 1, i32 0, i1 false, ptr @R.str)
1517
%h2 = call target("dx.TypedBuffer", float, 1, 0, 0) @llvm.dx.resource.handlefrombinding(i32 10, i32 5, i32 1, i32 0, i1 false, ptr @S.str)
1618
ret void

‎llvm/test/CodeGen/DirectX/Binding/binding-overlap-3.ll‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,17 @@
44
; Check multiple overlap errors.
55
; Also check different resource class with same binding values is ok (no error expected).
66

7-
target triple = "dxil-pc-shadermodel6.3-library"
7+
; C overlaps with A
8+
; C overlaps with B
9+
; StructuredBuffer<float> A : register(t5);
10+
; StructuredBuffer<float> B : register(t9);
11+
; StructuredBuffer<float> C[10] : register(t0);
12+
; RWBuffer<float> S[10] : register(u0);
13+
14+
; CHECK: error: resource C at register 0 overlaps with resource A at register 5 in space 0
15+
; CHECK: error: resource C at register 0 overlaps with resource B at register 9 in space 0
816

9-
; CHECK: error: resource C at register 0 overlaps with resource A at register 5, space 0
10-
; CHECK: error: resource C at register 0 overlaps with resource B at register 9, space 0
17+
target triple = "dxil-pc-shadermodel6.3-library"
1118

1219
@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
1320
@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
@@ -23,11 +30,6 @@ target triple = "dxil-pc-shadermodel6.3-library"
2330

2431
define void @test_overlapping() "hlsl.export" {
2532
entry:
26-
; StructuredBuffer<float> A : register(t5);
27-
; StructuredBuffer<float> B : register(t9);
28-
; StructuredBuffer<float> C[10] : register(t0);
29-
; RWBuffer<float> S[10] : register(u0);
30-
3133
%h1 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 5, i32 1, i32 0, i1 false, ptr @A.str)
3234
store target("dx.RawBuffer", float, 0, 0) %h1, ptr @One, align 4
3335

‎llvm/test/CodeGen/DirectX/Binding/binding-overlap-4.ll‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
; Use llc for this test so that we don't abort after the first error.
2+
; RUN: not llc %s -o /dev/null 2>&1 | FileCheck %s
3+
4+
; Check multiple overlap errors.
5+
6+
; A overlaps with B
7+
; A overlaps with C
8+
; B overlaps with C
9+
; StructuredBuffer<float> A[5] : register(t1); // 1-5
10+
; StructuredBuffer<float> B[2] : register(t2); // 2-3
11+
; StructuredBuffer<float> C[3] : register(t3); // 3-5
12+
13+
; CHECK: error: resource A at register 1 overlaps with resource B at register 2 in space 0
14+
; CHECK: error: resource A at register 1 overlaps with resource C at register 3 in space 0
15+
; CHECK: error: resource B at register 2 overlaps with resource C at register 3 in space 0
16+
17+
target triple = "dxil-pc-shadermodel6.3-library"
18+
19+
@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
20+
@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
21+
@C.str = private unnamed_addr constant [2 x i8] c"C\00", align 1
22+
23+
; Fake globals to store handles in; this is to make sure the handlefrombinding calls
24+
; are not optimized away by llc.
25+
@One = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
26+
@Two = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
27+
@Three = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
28+
29+
define void @test_overlapping() "hlsl.export" {
30+
entry:
31+
%h1 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 1, i32 5, i32 0, i1 false, ptr @A.str)
32+
store target("dx.RawBuffer", float, 0, 0) %h1, ptr @One, align 4
33+
34+
%h2 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 2, i32 2, i32 0, i1 false, ptr @B.str)
35+
store target("dx.RawBuffer", float, 0, 0) %h2, ptr @Two, align 4
36+
37+
%h3 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 0, i32 3, i32 3, i32 4, i1 false, ptr @C.str)
38+
store target("dx.RawBuffer", float, 0, 0) %h3, ptr @Three, align 4
39+
40+
ret void
41+
}

‎llvm/test/CodeGen/DirectX/Binding/binding-overlap-5.ll‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
; Use llc for this test so that we don't abort after the first error.
2+
; RUN: not llc %s -o /dev/null 2>&1 | FileCheck %s
3+
4+
; Check multiple overlap errors.
5+
6+
; A overlaps with B
7+
; B overlaps with C
8+
; StructuredBuffer<float> A[5] : register(t1, space11); // 1-5
9+
; StructuredBuffer<float> B[6] : register(t2, space11); // 2-7
10+
; StructuredBuffer<float> C[3] : register(t6, space11); // 6-8
11+
12+
; CHECK: error: resource A at register 1 overlaps with resource B at register 2 in space 11
13+
; CHECK: error: resource B at register 2 overlaps with resource C at register 6 in space 11
14+
15+
target triple = "dxil-pc-shadermodel6.3-library"
16+
17+
@A.str = private unnamed_addr constant [2 x i8] c"A\00", align 1
18+
@B.str = private unnamed_addr constant [2 x i8] c"B\00", align 1
19+
@C.str = private unnamed_addr constant [2 x i8] c"C\00", align 1
20+
21+
; Fake globals to store handles in; this is to make sure the handlefrombinding calls
22+
; are not optimized away by llc.
23+
@One = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
24+
@Two = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
25+
@Three = internal global { target("dx.RawBuffer", float, 0, 0) } poison, align 4
26+
27+
define void @test_overlapping() "hlsl.export" {
28+
entry:
29+
%h1 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 11, i32 1, i32 5, i32 0, i1 false, ptr @A.str)
30+
store target("dx.RawBuffer", float, 0, 0) %h1, ptr @One, align 4
31+
32+
%h2 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 11, i32 2, i32 6, i32 0, i1 false, ptr @B.str)
33+
store target("dx.RawBuffer", float, 0, 0) %h2, ptr @Two, align 4
34+
35+
%h3 = call target("dx.RawBuffer", float, 0, 0) @llvm.dx.resource.handlefrombinding(i32 11, i32 6, i32 3, i32 4, i1 false, ptr @C.str)
36+
store target("dx.RawBuffer", float, 0, 0) %h3, ptr @Three, align 4
37+
38+
ret void
39+
}

0 commit comments

Comments
 (0)