<div dir="ltr">Am Mi., 1. Juli 2020 um 12:24 Uhr schrieb Robinson, Paul <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>>:<br>><br>> What I actually meant re. CHECK-DAG is to take this<br>><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>><br>> and turn it into this<br>><br>>     72 ; CHECK: ![[ACCESS_GROUP_LIST_3]] = !{![[ACCESS_GROUP_INNER:[0-9]+]], ![[ACCESS_GROUP_OUTER:[0-9]+]]}<br>>     73 ; CHECK-DAG: ![[ACCESS_GROUP_INNER]] = distinct !{}<br>>     74 ; CHECK-DAG: ![[ACCESS_GROUP_OUTER]] = distinct !{}<br><br>Note that `CHECK-DAG: !{{[0-9]+}} = distinct !{}` could match multiple lines, requiring us find ACCESS_GROUP_INNER/ACCESS_GROUP_OUTER beforehand, making us check more than we may want to.<br><br><br>> except that I think I was misled by the “non-semantic” remark about the change. Did you mean that the order of the INNER and OUTER elements (line 72) has been swapped?  That sounds semantic, as far as the structure of the metadata is concerned!<br><br>Maybe we should clarify what I mean with "non-semantic":<br><br>Textual change: Some character in a line change<br>Structural change: The graph of MDNodes changes<br>Semantic change: There is a difference in what the MDNode means; depends on where the MDNode is used.<br><br>(I am not always consistent either)<br><br>The example patch is a structural, but non-semantic change: The order of operands in one MDNode is changed, but semantically the change is meaningless: To determine whether a loop is parallel, only containment in the access group list matters, but not the order in the list. An interesting property in the parallel-loop-md-merge.ll is that although there is a structural change, there is no textual change of that node. parallel-loop-md-merge.ll tries to test semantics using the text output.<br><br><br>> But okay, let’s call that a syntactic change, and a test relying on the order of the parameters will break.  Which it did.  And the correct fix is instead<br>><br>>     72 ; CHECK: ![[ACCESS_GROUP_LIST_3]] = !{![[ACCESS_GROUP_OUTER:[0-9]+]], ![[ACCESS_GROUP_INNER:[0-9]+]]}<br>><br>> is it not?  To reflect the change in order?<br><br>The test should not break in the first place since the change is insignificant.<div>Every spurious test fail adds to the burden of changing something, and it is easy to break hundreds of regression tests.<br><br><br>> But let’s say I’m the one doing this presumably innocuous change, and have no clue what I’m doing, and don’t know much about how FileCheck works (which is pretty typical of the community, I admit).  You’ve shown issues with trying to diagnose the FileCheck results.<br>><br>> How would the compiled regression test fail?  How would it be easier to identify and repair the issue?</div><div><br></div><div>It would not fail in the first place (admittedly this requires a well-designed test which I think is easier to write using LLVM's API directly).</div><div><br></div><div>If it does fail, it should be on semantic changes only. The called API directly points to what it is checking, in this case isAnnotatedParallel():</div><div><pre style="font-family:Consolas;font-size:13px;color:black;background-image:initial;background-position:initial;background-size:initial;background-repeat:initial;background-origin:initial;background-clip:initial"><span style="color:rgb(60,60,60)">  /// Returns true if the loop is annotated parallel.</span>
  <span style="color:rgb(60,60,60)">///</span>
  <span style="color:rgb(60,60,60)">/// A parallel loop can be assumed to not contain any dependencies between</span>
  <span style="color:rgb(60,60,60)">/// iterations by the compiler. That is, any loop-carried dependency checking</span>
  <span style="color:rgb(60,60,60)">/// can be skipped completely when parallelizing the loop on the target</span>
  <span style="color:rgb(60,60,60)">/// machine. Thus, if the parallel loop information originates from the</span>
  <span style="color:rgb(60,60,60)">/// programmer, e.g. via the OpenMP parallel for pragma, it is the</span>
  <span style="color:rgb(60,60,60)">/// programmer's responsibility to ensure there are no loop-carried</span>
  <span style="color:rgb(60,60,60)">/// dependencies. The final execution order of the instructions across</span>
  <span style="color:rgb(60,60,60)">/// iterations is not guaranteed, thus, the end result might or might not</span>
  <span style="color:rgb(60,60,60)">/// implement actual concurrent execution of instructions across multiple</span>
  <span style="color:rgb(60,60,60)">/// iterations.</span>
  <span style="font-weight:bold">bool</span> <span style="color:blue">isAnnotatedParallel</span>() <span style="font-weight:bold">const</span>;
</pre></div><div>which should help you determine whether a patch intends to have this effect or isAnnotatedParallel() should be fixed. In this case, fixing isAnnotatedParallel fixes the regression tests as well as LLVM itself.</div><div><br></div><div><br></div><div><br>> Re. how MIR makes testing easier: it is a serialization of the data that a machine-IR pass operates on, which makes it feasible to feed canned MIR through a single pass in llc and look at what exactly that pass did.  Prior to MIR, we had to go from IR to real machine code and infer what was going on in a pass after multiple levels of transformation had occurred.  It was very black-box, and the black box was way too big.<br><br></div><div>mlir-opt ?</div><div><br></div><div><br></div><div>Michael</div><div><br></div></div>