<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><br></div><div><div>On Oct 7, 2013, at 3:42 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">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 class="Apple-tab-span" style="white-space:pre"> </span>str<span class="Apple-tab-span" style="white-space:pre"> </span>r0, [sp, #8]</div><div>+;CHECK-DAG: <span class="Apple-tab-span" style="white-space:pre"> </span>add<span class="Apple-tab-span" style="white-space:pre"> </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>Yes that was too much -DAG adding on my part, I revised the patch. This place looks like this now:</div><div><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>so the 3rd instruction can still move anywhere in between but the 2nd should have a dependency on the first one now.</div></div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><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>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><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><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>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><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><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>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><br></div><div>Thanks for the review,</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Matthias</div><div><br></div><div></div></div></body></html>