<div dir="ltr">LGTM.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On 10 October 2013 23:33, Matthias Braun <span dir="ltr"><<a href="mailto:mbraun@apple.com" target="_blank">mbraun@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><br></div><div><div class="im"><div>On Oct 7, 2013, at 3:42 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" target="_blank">qcolombet@apple.com</a>> wrote:</div>
<br><blockquote type="cite"><div style="word-wrap:break-word">Hi Matthias,<div><br></div><div>0001:</div><div>LGTM.</div><div><br></div><div>0002:</div><div><div>Unless I am missing the semantic of CHECK-DAG, this seems wrong:</div>
<div>+;CHECK-DAG: <span style="white-space:pre-wrap"> </span>str<span style="white-space:pre-wrap"> </span>r0, [sp, #8]</div><div>+;CHECK-DAG: <span style="white-space:pre-wrap"> </span>add<span style="white-space:pre-wrap"> </span>r0, sp, #8</div>
<div>How could a str-add sequence with a write-after-read dependency be semantically equivalent to a add-str with a read-after-write dependency?</div></div></div></blockquote></div><div>Yes that was too much -DAG adding on my part, I revised the patch. This place looks like this now:</div>
<div><div class="im"><div>+;CHECK-DAG: str [[R0:r0]], [sp, #8]</div><div>+;CHECK-DAG: add [[R0]], sp, #8</div><div>+;CHECK-DAG: str r2, [sp, #12]</div></div><div>so the 3rd instruction can still move anywhere in between but the 2nd should have a dependency on the first one now.</div>
</div><div class="im"><br><blockquote type="cite"><div style="word-wrap:break-word"><div><div><br></div><div>At several places, you’ve replaced:</div><div>r0, r1, etc.</div><div>by</div><div>{{r[0-9]+}}</div><div>I didn’t check all the places, but in some of them, r0, r1, etc. directly come from the ABI constraints, so we want to explicitly check for them.</div>
<div>Please make sure, you are not obfuscated ABI constraints as it may emphasize we are doing something wrong!</div><div>One example is</div><div>-;CHECK: vst1.16 {d16[2]}, [r0:16]</div><div>+;CHECK: vst1.16 {{{d[0-9]+}}[2]}, [{{r[0-9]+}}:16]</div>
<div>r0 comes from the ABI.</div></div></div></blockquote></div><div>yes right all the places with r0 accessed paramters, I reverted this. All the _update tests that mentioned r1 did not access an ABI parameter and I left them as is.</div>
<div class="im"><br><blockquote type="cite"><div style="word-wrap:break-word"><div><div><br></div><div>Or here</div><div><div>-;CHECK: vst2.16 {d16[1], d17[1]}, [r0:32]</div><div>+;CHECK: vst2.16 {d16[1], d17[1]}, [{{r[0-9]+}}:32]</div>
</div><div>This one is even worse, because r0 comes from the ABI but not d16 and d17, thus they should have been “regexped” but not r0.</div></div></div></blockquote></div><div>I wasn’t entirely sure if d16+d17 has to be in order or not, so I left them intact, I did not replace r0 with a regex in the updated version anymore.</div>
<div class="im"><br><blockquote type="cite"><div style="word-wrap:break-word"><div><div><br></div><div>The last one should use CHECK and not CHECK-DAG:</div><div><div>+; CHECK-DAG: add.w r[[ADDR:[0-9]+]], r[[SOURCE]], {{r[0-9]+}}, lsl #2</div>
<div>+; CHECK-DAG: vld1.32 {[[DREG:d[0-9]+]][], [[DREG2:d[0-9]+]][]}, [r[[ADDR]]:32] <— ADDR, read-after-write dependencies.</div></div></div></div></blockquote></div><div>FileCheck is smart enough to compute dependencies between [[Variables]] and enforce a correct order, but for this case it didn’t change anything compared to just CHECK: so I reverted that part.</div>
<div class="im"><div><br></div><div>Thanks for the review,</div><div><span style="white-space:pre-wrap"> </span>Matthias</div><div><br></div><div></div></div></div></div><br><div style="word-wrap:break-word"><div><div></div>
<br><blockquote type="cite"><div style="word-wrap:break-word"><div><div><br></div><div>0003:</div><div>LGTM.</div><div> </div><div>Thanks,</div><div>
<div style="font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
-Quentin</div>
</div>
<br><div><div>On Oct 7, 2013, at 2:08 PM, Matthias Braun <<a href="mailto:mbraun@apple.com" target="_blank">mbraun@apple.com</a>> wrote:</div><br><blockquote type="cite">Attached are a few improvements to make some (mostly arm) testcases less dependent on a particular schedule/register allocation. Maybe you want to have these changes in the repository. Descriptions:<br>
<br>* Tests: Use CHECK-LABEL where possible<br>* Tests: Be less dependent on a specific schedule/regalloc<br>* Tests: Do not unnecessarily depend on kill comments<br><br>Greetings<br><span style="white-space:pre-wrap"> </span>Matthias<br>
<br><span><0001-Tests-Use-CHECK-LABEL-where-possible.patch></span><span><0002-Tests-Be-less-dependent-on-a-specific-schedule-regal.patch></span><span><0003-Tests-Do-not-unnecessarily-depend-on-kill-comments.patch></span>_______________________________________________<br>
llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div></blockquote></div><br></div><br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>