<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:677389290;
mso-list-template-ids:-111512560;}
@list l0:level1
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:.5in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Symbol;}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:1.0in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:"Courier New";
mso-bidi-font-family:"Times New Roman";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:1.5in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:2.0in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:2.5in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:3.0in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:3.5in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:4.0in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:4.5in;
mso-level-number-position:left;
text-indent:-.25in;
mso-ansi-font-size:10.0pt;
font-family:Wingdings;}
ol
{margin-bottom:0in;}
ul
{margin-bottom:0in;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal">The test as written is fragile because it requires a certain ordering. If the output order is not important, use CHECK-DAG rather than CHECK. This would be a failure to understand the testing tool.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">My experience, over a 40-year career, is that good software developers are generally not very good test-writers. These are different skills and good testing is frequently not taught. It’s easy to write fragile tests; you make your patch,
you see what comes out, and you write the test to expect exactly that output, using the minimum possible features of the testing tool. This is poor technique all around. We even have scripts that automatically generate such tests, used primarily in codegen
tests. I devoutly hope that the people who produce those tests responsibly eyeball all those cases.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">The proposal appears to be to migrate output-based tests (using ever-more-complicated FileCheck features) to executable tests, which makes it more like the software development people are used to instead of test-writing. But I don’t see
that necessarily solving the problem; seems like it would be just as easy to write a program that doesn’t do the right thing as to write a FileCheck test that doesn’t do the right thing.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Hal’s suggestion is more to the point: If the output we’re generating is not appropriate to the kinds of tests we want to perform, it can be worthwhile to generate different kinds of output. MIR is a case in point; for a long time it
was hard to introspect into the interval between IR and final machine code, but now it’s a lot easier.<o:p></o:p></p>
<p class="MsoNormal">--paulr<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> llvm-dev <llvm-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Michael Kruse via llvm-dev<br>
<b>Sent:</b> Wednesday, July 1, 2020 1:40 AM<br>
<b>To:</b> Michael Kruse <llvmdev@meinersbur.de><br>
<b>Cc:</b> llvm-dev <llvm-dev@lists.llvm.org><br>
<b>Subject:</b> Re: [llvm-dev] [RFC] Compiled regression tests.<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">To illustrate some difficulties with FileCheck, lets make a non-semantic change in LLVM:<br>
<br>
<span style="font-family:"Courier New""> --- a/llvm/lib/Analysis/VectorUtils.cpp<br>
+++ b/llvm/lib/Analysis/VectorUtils.cpp<br>
@@ -642,8 +642,8 @@ MDNode *llvm::uniteAccessGroups(MDNode *AccGroups1, MDNode *AccGroups2) {<br>
return AccGroups1;<br>
<br>
SmallSetVector<Metadata *, 4> Union;<br>
- addToAccessGroupList(Union, AccGroups1);<br>
addToAccessGroupList(Union, AccGroups2);<br>
+ addToAccessGroupList(Union, AccGroups1);<br>
<br>
if (Union.size() == 0)<br>
return nullptr;</span><br>
<br>
This changes the order of access groups when merging them.<br>
Same background: Access groups are used to mark that a side-effect (e.g. memory access) does not limit the parallelization of a loop, added by e.g. #pragma clang loop vectorize(assume_safety). Therefore a loop can be assumed to be parallelizable without further
dependency analysis if all its side-effect instructions are marked this way (and has no loop-carried scalar dependencies).<br>
An access group represents the instructions marked for a specific loop. A single instruction can be parallel with regrads to multiple loop, i.e. belong to multiple access groups. The order of the access groups is insignificant, i.e. whether either `AccGroups1`
or `AccGroups2` comes first, the instruction is marked parallel for both loops.<br>
<br>
Even the change should have no effect on correctness, `ninja check-llvm` fails:<br>
<br>
test/Transforms/Inline/parallel-loop-md-merge.ll<br>
<br>
<span style="font-family:"Courier New""> string not found in input<br>
; CHECK: ![[ACCESSES_INNER]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_INNER]]}<br>
^<br>
<stdin>:45:1: note: scanning from here<br>
!7 = !{!"llvm.loop.parallel_accesses", !5}<br>
^<br>
<stdin>:45:1: note: with "ACCESSES_INNER" equal to "7"<br>
!7 = !{!"llvm.loop.parallel_accesses", !5}<br>
^<br>
<stdin>:45:1: note: with "ACCESS_GROUP_INNER" equal to "4"<br>
!7 = !{!"llvm.loop.parallel_accesses", !5}<br>
^</span><br>
<br>
The line FileCheck points to has nothing to do with the changed order of access groups.<br>
(what's the point of annotating the `!7 = ...` line repeatedly? Why not pointing to the lines where ACCESSES_INNER/ACCESS_GROUP_INNER have their values from?)<br>
<br>
The CHECK lines, annotated with their line numbers from parallel-loop-md-merge.ll, are:<br>
<br>
<span style="font-family:"Courier New""> 68 ; CHECK: store double 4.200000e+01, {{.*}} !llvm.access.group ![[ACCESS_GROUP_LIST_3:[0-9]+]]<br>
69 ; CHECK: br label %for.cond.i, !llvm.loop ![[LOOP_INNER:[0-9]+]]<br>
70 ; CHECK: br label %for.cond, !llvm.loop ![[LOOP_OUTER:[0-9]+]]<br>
71<br>
72 ; CHECK: ![[ACCESS_GROUP_LIST_3]] = !{![[ACCESS_GROUP_INNER:[0-9]+]], ![[ACCESS_GROUP_OUTER:[0-9]+]]}<br>
73 ; CHECK: ![[ACCESS_GROUP_INNER]] = distinct !{}<br>
74 ; CHECK: ![[ACCESS_GROUP_OUTER]] = distinct !{}<br>
75 ; CHECK: ![[LOOP_INNER]] = distinct !{![[LOOP_INNER]], ![[ACCESSES_INNER:[0-9]+]]}<br>
76 ; CHECK: ![[ACCESSES_INNER]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_INNER]]}<br>
77 ; CHECK: ![[LOOP_OUTER]] = distinct !{![[LOOP_OUTER]], ![[ACCESSES_OUTER:[0-9]+]]}<br>
78 ; CHECK: ![[ACCESSES_OUTER]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_OUTER]]}</span><br>
<br>
And the output of `FileCheck --dump-input -v` is:<br>
<br>
<span style="font-family:"Courier New""> 38: !0 = !{!1}<br>
39: !1 = distinct !{!1, !2, !"callee: %A"}<br>
40: !2 = distinct !{!2, !"callee"}<br>
41: !3 = !{!4, !5}<br>
check:72 ^~~~~~~~~~~~~~<br>
42: !4 = distinct !{}<br>
check:73 ^~~~~~~~~~~~~~~~~<br>
43: !5 = distinct !{}<br>
check:74 ^~~~~~~~~~~~~~~~~<br>
44: !6 = distinct !{!6, !7}<br>
check:75 ^~~~~~~~~~~~~~~~~~~~~~~<br>
45: !7 = !{!"llvm.loop.parallel_accesses", !5}<br>
check:76 X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found<br>
46: !8 = distinct !{!8, !9}<br>
check:76 ~~~~~~~~~~~~~~~~~~~~~~~<br>
47: !9 = !{!"llvm.loop.parallel_accesses", !4}<br>
check:76 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~</span><br>
<br>
To fix this regression test, one needs to solve the following questions:<o:p></o:p></p>
<ul type="disc">
<li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
Is `!0 = !{!1}` supposed to match with anything?<o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
Which of the CHECK lines are the ones that are important for the regression tests?<o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
The `![[SOMETHING]] = distinct !{}` line appears twice. Which one is ACCESS_GROUP_INNER/ACCESS_GROUP_OUTER? <o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
There are two lines with `llvm.loop.parallel_accesses` as well. The author of this regression test was kind enough to give the placeholders descriptive names, but do they match with what FileCheck matched? <o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
LOOP_INNER and LOOP_OUTER are only distinguished by position and `.i`. Why does the inner loop come first? <o:p></o:p></li><li class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto;mso-list:l0 level1 lfo1">
Before the patch that modifies the order, the output of --dump-input=always was:<o:p></o:p></li></ul>
<p class="MsoNormal"><span style="font-family:"Courier New""> 38: !0 = !{!1}<br>
39: !1 = distinct !{!1, !2, !"callee: %A"}<br>
40: !2 = distinct !{!2, !"callee"}<br>
41: !3 = !{!4, !5}<br>
42: !4 = distinct !{}<br>
43: !5 = distinct !{}<br>
44: !6 = distinct !{!6, !7}<br>
45: !7 = !{!"llvm.loop.parallel_accesses", !4}<br>
46: !8 = distinct !{!8, !9}<br>
47: !9 = !{!"llvm.loop.parallel_accesses", !5}</span><br>
<br>
This seems to suggest that our change caused !4 and !5 in lines 45 and 47 to swap. This is not what the patch did, which changes to order of two MDNode arguments. The only MDNode that has two other MDNodes as operands is line (line !6 and !8 are representing
loops as they reference themselves as first arguments; noalias scopes do this as well, but there is no noalias metadata in this example).<br>
<br>
An hasty fix is to change CHECK lines 76 and 77 to<br>
<span style="font-family:"Courier New""><br>
76 ; CHECK: ![[ACCESSES_INNER]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_OUTER]]}<br>
78 ; CHECK: ![[ACCESSES_OUTER]] = !{!"llvm.loop.parallel_accesses", ![[ACCESS_GROUP_INNER]]}</span><br>
<br>
However, ACCESS_GROUP_OUTER belongs to ACCESSES_INNER and ACCESS_GROUP_INNER to ACCESSES_OUTER? This cannot be true.<br>
<br>
<br>
The reason is that MDNodes are emitted in topological order of a depth-first search. The patch changed the order of ACCESS_GROUP_INNER and ACCESS_GROUP_OUTER in line 41 but because in the mitted IR file, the first argument of !3 is emitted before the second
argument, ACCESS_GROUP_OUTER is on line 42 and ACCESS_GROUP_INNER on line 43 instead.<br>
<br>
Even though this regression test uses proper regex-ing of metadata node numbers, it is still fragile as it fails on irrelevant changes. Once it fails, it requires time to fix properly because it is non-obvious which lines are intended to match which output
lines. The hasty fix would have mixed up ACCESS_GROUP_OUTER/ACCESS_GROUP_INNER and I wouldn't expect someone who is working on the inliner to take the time to understand all the loop metadata. Set-semantics of metadata occurs often, such as noalias metadata,
loop properties, tbaa metadata, etc.<br>
<br>
The difficulty gets amplified when there are multiple functions in the regression tests; metadata of all functions all appended to the end and MDNodes with same content uniqued, making it impossible to associate particular MDNodes with specific functions.<br>
<br>
<br>
Ideally the regression test would be robust and understandable, achievable with two asserts in a unittest:<br>
<br>
<span style="font-family:"Courier New""> Loop &OuterLoop = *LI->begin();<br>
ASSERT_TRUE(OuterLoop.isAnnotatedParallel());<br>
Loop &InnerLoop = *OuterLoop.begin();<br>
ASSERT_TRUE(InnerLoop.isAnnotatedParallel());</span><o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>