Skip to content

Commit 062c7b6

Browse files
inbelicinbelic
committed
self-review: strip all unrecognized metadata instead of errors
1 parent 24909ab commit 062c7b6

File tree

5 files changed

+139
-150
lines changed

5 files changed

+139
-150
lines changed

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

Lines changed: 71 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -327,81 +327,90 @@ static void translateBranchMetadata(Module &M, Instruction *BBTerminatorInst) {
327327
BBTerminatorInst->setMetadata("hlsl.controlflow.hint", nullptr);
328328
}
329329

330-
static void translateLoopMetadata(Module &M, MDNode *LoopMD) {
330+
// Determines if the metadata node will be compatible with DXIL's loop metadata
331+
// representation.
332+
//
333+
// Reports an error for compatible metadata that is ill-formed.
334+
static bool isLoopMDCompatible(Module &M, Metadata *MD) {
331335
// DXIL only accepts the following loop hints:
332-
// llvm.loop.unroll.disable, llvm.loop.unroll.full, llvm.loop.unroll.count
333336
std::array<StringLiteral, 3> ValidHintNames = {"llvm.loop.unroll.count",
334337
"llvm.loop.unroll.disable",
335338
"llvm.loop.unroll.full"};
336339

337-
// llvm.loop metadata must have its first operand be a self-reference, so we
338-
// require at least 1 operand.
339-
//
340-
// It only makes sense to specify up to 1 of the hints on a branch, so we can
341-
// have at most 2 operands.
340+
MDNode *HintMD = dyn_cast<MDNode>(MD);
341+
if (!HintMD || HintMD->getNumOperands() == 0)
342+
return false;
342343

343-
if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) {
344-
reportLoopError(M, "Requires exactly 1 or 2 operands");
345-
return;
346-
}
344+
auto *HintStr = dyn_cast<MDString>(HintMD->getOperand(0));
345+
if (!HintStr)
346+
return false;
347347

348-
if (LoopMD != LoopMD->getOperand(0)) {
349-
reportLoopError(M, "First operand must be a self-reference");
350-
return;
351-
}
348+
if (!llvm::is_contained(ValidHintNames, HintStr->getString()))
349+
return false;
352350

353-
// A node only containing a self-reference is a valid use to denote a loop
354-
if (LoopMD->getNumOperands() == 1)
355-
return;
351+
auto ValidCountNode = [](MDNode *CountMD) -> bool {
352+
if (CountMD->getNumOperands() == 2)
353+
if (auto *Count = dyn_cast<ConstantAsMetadata>(CountMD->getOperand(1)))
354+
if (isa<ConstantInt>(Count->getValue()))
355+
return true;
356+
return false;
357+
};
356358

357-
LoopMD = dyn_cast<MDNode>(LoopMD->getOperand(1));
358-
if (!LoopMD) {
359-
reportLoopError(M, "Second operand must be a metadata node");
360-
return;
361-
}
359+
if (HintStr->getString() == "llvm.loop.unroll.count") {
360+
if (!ValidCountNode(HintMD))
361+
return reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" "
362+
"must be a constant integer");
363+
} else if (HintMD->getNumOperands() != 1)
364+
return reportLoopError(
365+
M, "\"llvm.loop.unroll.disable\" and \"llvm.loop.unroll.disable\" "
366+
"must be provided as a single operand");
362367

363-
if (LoopMD->getNumOperands() != 1 && LoopMD->getNumOperands() != 2) {
364-
reportLoopError(M, "Requires exactly 1 or 2 operands");
365-
return;
366-
}
368+
return true;
369+
}
370+
371+
static void translateLoopMetadata(Module &M, Instruction *I, MDNode *BaseMD) {
372+
// A distinct node has the self-referential form: !0 = !{ !0, ... }
373+
auto IsDistinctNode = [](MDNode *Node) -> bool {
374+
return Node && Node->getNumOperands() != 0 && Node == Node->getOperand(0);
375+
};
376+
377+
// Strip empty metadata or a non-distinct node
378+
if (BaseMD->getNumOperands() == 0 || !IsDistinctNode(BaseMD))
379+
return I->setMetadata("llvm.loop", nullptr);
367380

368-
// It is valid to have a chain of self-referential loop metadata nodes so if
369-
// we have another self-reference, recurse.
381+
// It is valid to have a chain of self-refential loop metadata nodes, as
382+
// below. We will collapse these into just one when we reconstruct the
383+
// metadata.
370384
//
371385
// Eg:
372386
// !0 = !{!0, !1}
373387
// !1 = !{!1, !2}
374-
// !2 = !{"llvm.loop.unroll.disable"}
375-
if (LoopMD == LoopMD->getOperand(0))
376-
return translateLoopMetadata(M, LoopMD);
377-
378-
// Otherwise, we are at our base hint metadata node
379-
auto *HintStr = dyn_cast<MDString>(LoopMD->getOperand(0));
380-
if (!HintStr || !llvm::is_contained(ValidHintNames, HintStr->getString())) {
381-
reportLoopError(M,
382-
"First operand must be a valid \"llvm.loop.unroll\" hint");
383-
return;
384-
}
385-
386-
// Ensure count node is a constant integer value
387-
auto ValidCountNode = [](MDNode *HintMD) -> bool {
388-
if (HintMD->getNumOperands() == 2)
389-
if (auto *CountMD = dyn_cast<ConstantAsMetadata>(HintMD->getOperand(1)))
390-
if (isa<ConstantInt>(CountMD->getValue()))
391-
return true;
392-
return false;
393-
};
394-
395-
if (HintStr->getString() == "llvm.loop.unroll.count") {
396-
if (!ValidCountNode(LoopMD)) {
397-
reportLoopError(M, "Second operand of \"llvm.loop.unroll.count\" "
398-
"must be a constant integer");
399-
return;
400-
}
401-
} else if (LoopMD->getNumOperands() != 1) {
402-
reportLoopError(M, "Can't have a second operand");
403-
return;
404-
}
388+
// !2 = !{!"llvm.loop.unroll.disable"}
389+
//
390+
// So, traverse down a potential self-referential chain
391+
while (1 < BaseMD->getNumOperands() &&
392+
IsDistinctNode(dyn_cast<MDNode>(BaseMD->getOperand(1))))
393+
BaseMD = dyn_cast<MDNode>(BaseMD->getOperand(1));
394+
395+
// To reconstruct a distinct node we create a temporary node that we will
396+
// then update to create a self-reference.
397+
llvm::TempMDTuple TempNode = llvm::MDNode::getTemporary(M.getContext(), {});
398+
SmallVector<Metadata *> CompatibleOperands = {TempNode.get()};
399+
400+
// Iterate and reconstruct the metadata nodes that contains any hints,
401+
// stripping any unrecognized metadata.
402+
ArrayRef<MDOperand> Operands = BaseMD->operands();
403+
for (auto &Op : Operands.drop_front())
404+
if (isLoopMDCompatible(M, Op.get()))
405+
CompatibleOperands.push_back(Op.get());
406+
407+
if (2 < CompatibleOperands.size())
408+
reportLoopError(M, "Provided conflicting hints");
409+
410+
MDNode *CompatibleLoopMD = MDNode::get(M.getContext(), CompatibleOperands);
411+
TempNode->replaceAllUsesWith(CompatibleLoopMD);
412+
413+
I->setMetadata("llvm.loop", CompatibleLoopMD);
405414
}
406415

407416
using InstructionMDList = std::array<unsigned, 7>;
@@ -427,10 +436,9 @@ static void translateInstructionMetadata(Module &M) {
427436
translateBranchMetadata(M, I);
428437

429438
for (auto &I : make_early_inc_range(BB)) {
430-
if (isa<BranchInst>(I)) {
439+
if (isa<BranchInst>(I))
431440
if (MDNode *LoopMD = I.getMetadata(MDLoopKind))
432-
translateLoopMetadata(M, LoopMD);
433-
}
441+
translateLoopMetadata(M, &I, LoopMD);
434442
I.dropUnknownNonDebugMetadata(DXILCompatibleMDs);
435443
}
436444
}

‎llvm/test/CodeGen/DirectX/Metadata/loop-md-errs.ll‎

Lines changed: 6 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
; RUN: split-file %s %t
22
; RUN: not opt -S --dxil-translate-metadata %t/args.ll 2>&1 | FileCheck %t/args.ll
3-
; RUN: not opt -S --dxil-translate-metadata %t/not-ref.ll 2>&1 | FileCheck %t/not-ref.ll
4-
; RUN: not opt -S --dxil-translate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll
5-
; RUN: not opt -S --dxil-translate-metadata %t/not-str.ll 2>&1 | FileCheck %t/not-str.ll
63
; RUN: not opt -S --dxil-translate-metadata %t/bad-count.ll 2>&1 | FileCheck %t/bad-count.ll
74
; RUN: not opt -S --dxil-translate-metadata %t/invalid-disable.ll 2>&1 | FileCheck %t/invalid-disable.ll
85
; RUN: not opt -S --dxil-translate-metadata %t/invalid-full.ll 2>&1 | FileCheck %t/invalid-full.ll
@@ -11,7 +8,7 @@
118

129
;--- args.ll
1310

14-
; CHECK: Invalid "llvm.loop" metadata: Requires exactly 1 or 2 operands
11+
; CHECK: Invalid "llvm.loop" metadata: Provided conflicting hints
1512

1613
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
1714

@@ -32,83 +29,9 @@ exit:
3229
ret void
3330
}
3431

35-
!1 = !{!1, !1, !1} ; too many args
36-
37-
;--- not-ref.ll
38-
39-
; CHECK: Invalid "llvm.loop" metadata: First operand must be a self-reference
40-
41-
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
42-
43-
define void @example_loop(i32 %n) {
44-
entry:
45-
br label %loop.header
46-
47-
loop.header:
48-
%i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ]
49-
%cmp = icmp slt i32 %i, %n
50-
br i1 %cmp, label %loop.body, label %exit
51-
52-
loop.body:
53-
%i.next = add nsw i32 %i, 1
54-
br label %loop.header, !llvm.loop !1
55-
56-
exit:
57-
ret void
58-
}
59-
60-
!1 = !{i32 0} ; not a self-reference
61-
62-
;--- not-md.ll
63-
64-
; CHECK: Invalid "llvm.loop" metadata: Second operand must be a metadata node
65-
66-
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
67-
68-
define void @example_loop(i32 %n) {
69-
entry:
70-
br label %loop.header
71-
72-
loop.header:
73-
%i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ]
74-
%cmp = icmp slt i32 %i, %n
75-
br i1 %cmp, label %loop.body, label %exit
76-
77-
loop.body:
78-
%i.next = add nsw i32 %i, 1
79-
br label %loop.header, !llvm.loop !1
80-
81-
exit:
82-
ret void
83-
}
84-
85-
!1 = !{!1, i32 0} ; not a metadata node
86-
87-
;--- not-str.ll
88-
89-
; CHECK: Invalid "llvm.loop" metadata: First operand must be a valid "llvm.loop.unroll" hint
90-
91-
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
92-
93-
define void @example_loop(i32 %n) {
94-
entry:
95-
br label %loop.header
96-
97-
loop.header:
98-
%i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ]
99-
%cmp = icmp slt i32 %i, %n
100-
br i1 %cmp, label %loop.body, label %exit
101-
102-
loop.body:
103-
%i.next = add nsw i32 %i, 1
104-
br label %loop.header, !llvm.loop !1
105-
106-
exit:
107-
ret void
108-
}
109-
110-
!1 = !{!1, !2}
111-
!2 = !{i32 0} ; not a hint name string
32+
!1 = !{!1, !2, !3} ; conflicting args
33+
!2 = !{!"llvm.loop.unroll.full"}
34+
!3 = !{!"llvm.loop.unroll.disable"}
11235

11336
;--- bad-count.ll
11437

@@ -138,7 +61,7 @@ exit:
13861

13962
;--- invalid-disable.ll
14063

141-
; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand
64+
; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand
14265

14366
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
14467

@@ -165,7 +88,7 @@ exit:
16588

16689
;--- invalid-full.ll
16790

168-
; CHECK: Invalid "llvm.loop" metadata: Can't have a second operand
91+
; CHECK: Invalid "llvm.loop" metadata: "llvm.loop.unroll.disable" and "llvm.loop.unroll.disable" must be provided as a single operand
16992

17093
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
17194

‎llvm/test/CodeGen/DirectX/Metadata/loop-md-stripped.ll‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
; RUN: split-file %s %t
2+
; RUN: opt -S --dxil-translate-metadata %t/not-distinct.ll 2>&1 | FileCheck %t/not-distinct.ll
3+
; RUN: opt -S --dxil-translate-metadata %t/not-md.ll 2>&1 | FileCheck %t/not-md.ll
4+
5+
; Test that DXIL incompatible loop metadata is stripped
6+
7+
;--- not-distinct.ll
8+
9+
; Ensure it is stripped because it is not provided a distinct loop parent
10+
; CHECK-NOT: {!"llvm.loop.unroll.disable"}
11+
12+
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
13+
14+
define void @example_loop(i32 %n) {
15+
entry:
16+
br label %loop.header
17+
18+
loop.header:
19+
%i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ]
20+
%cmp = icmp slt i32 %i, %n
21+
br i1 %cmp, label %loop.body, label %exit
22+
23+
loop.body:
24+
%i.next = add nsw i32 %i, 1
25+
br label %loop.header, !llvm.loop !1
26+
27+
exit:
28+
ret void
29+
}
30+
31+
!1 = !{!"llvm.loop.unroll.disable"} ; first node must be a distinct self-reference
32+
33+
34+
;--- not-md.ll
35+
36+
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
37+
38+
define void @example_loop(i32 %n) {
39+
entry:
40+
br label %loop.header
41+
42+
loop.header:
43+
%i = phi i32 [ 0, %entry ], [ %i.next, %loop.body ]
44+
%cmp = icmp slt i32 %i, %n
45+
br i1 %cmp, label %loop.body, label %exit
46+
47+
loop.body:
48+
%i.next = add nsw i32 %i, 1
49+
; CHECK: br label %loop.header, !llvm.loop ![[#LOOP_MD:]]
50+
br label %loop.header, !llvm.loop !1
51+
52+
exit:
53+
ret void
54+
}
55+
56+
; CHECK: ![[#LOOP_MD:]] = distinct !{![[#LOOP_MD]]}
57+
58+
!1 = !{!1, i32 0} ; not a metadata node

‎llvm/test/CodeGen/DirectX/Metadata/loop-md-valid.ll‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
;--- count.ll
77

8-
; Test that we allow a self-referential chain and a constant integer in count
8+
; Test that we collapse a self-referential chain and allow a unroll.count hint
99

1010
target triple = "dxilv1.0-unknown-shadermodel6.0-library"
1111

@@ -27,8 +27,7 @@ exit:
2727
ret void
2828
}
2929

30-
; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#EXTRA_REF:]]}
31-
; CHECK: ![[#EXTRA_REF]] = distinct !{![[#EXTRA_REF]], ![[#COUNT:]]}
30+
; CHECK: ![[#LOOP_MD]] = distinct !{![[#LOOP_MD]], ![[#COUNT:]]}
3231
; CHECK: ![[#COUNT]] = !{!"llvm.loop.unroll.count", i6 4}
3332

3433
!0 = !{!0, !1}

‎llvm/test/CodeGen/DirectX/metadata-stripping.ll‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ _Z4mainDv3_j.exit: ; preds = %for.body.i, %entry
2525
; No more metadata should be necessary, the rest (the current 0 and 1)
2626
; should be removed.
2727
; CHECK-DAG: [[RANGEMD]] = !{i32 1, i32 5}
28+
; CHECK-DAG: [[LOOPMD]] = distinct !{[[LOOPMD]]}
2829
; CHECK-NOT: {!"llvm.loop.mustprogress"}
29-
!0 = distinct !{!0, !1}
30+
!0 = distinct !{!0, !1, !2}
3031
!1 = !{!"llvm.loop.mustprogress"}
3132
!2 = !{i32 1, i32 5}

0 commit comments

Comments
 (0)