<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;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 6, 2014, at 11:11 AM, Frédéric Riss <<a href="mailto:friss@apple.com" class="">friss@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=windows-1252" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div class=""><div class="">On 06 Aug 2014, at 19:50, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><blockquote type="cite" class=""><div style="font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><blockquote type="cite" class="">Is there<br class="">some more general test we can do here that would catch more cases of<br class="">MIs that don't have register definitions? (to have a bigger impact and<br class="">make the intent of the code clearer - it's not about skipping debug<br class="">values, it's about skipping non-defining instructions)<br class=""></blockquote><br class="">That’s a question I asked myself. If this test exists, then it could go into calculateDbgValueHistory also to get bigger impact.<br class=""></blockquote><br class="">Yeah - then it seems weird to have it out here. Just makes the reader<br class="">ask more questions, I think. Either it matters and it should be in<br class="">calculateDbgValueHistory, as you say, or it doesn't and it shouldn't<br class="">be anywhere because it's just adding cognitive load for readers...<br class=""><br class="">I'd just suggest dropping that (& pestering Alexey about why it was in<br class="">the other place too).<br class=""></div></blockquote></div><br class=""><div class="">Well, in calculateDbgValueHistory the test has to be there as the loop is meant to handle DbgValue instructions specifically.</div><div class=""><br class=""></div></div></div></blockquote><div><br class=""></div><div>It shouldn’t matter in collectChangingRegs(), because a debug value doesn’t define any registers. It’s probably marginally faster than going through the list of operands, but otherwise safe to drop. The other instance in calculateDbgValueHistory() needs to be there for the reason mentioned above. I’ll go ahead and commit this (without the first check) now.</div><div><br class=""></div><div>thanks,</div><div>adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Fred</div></div></div></blockquote></div><br class=""></body></html>