<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Mark, <div><br></div><div>Thanks for catching this. This change LGTM. </div><div><br></div><div>Nadav</div><div><br></div><div><div><div>On May 15, 2013, at 11:23 AM, Mark Seaborn <<a href="mailto:mseaborn@chromium.org">mseaborn@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: 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;"><div dir="ltr">Fix miscompile due to StackColoring incorrectly merging stack slots<br><br>IR optimisation passes can result in a basic block that contains:<br><br>  llvm.lifetime.start(%buf)<br>  ...<br>  llvm.lifetime.end(%buf)<br>  ...<br>  llvm.lifetime.start(%buf)<br><br>Before this change, calculateLiveIntervals() was ignoring the second<br>lifetime.start() and was regarding %buf as being dead from the<br>lifetime.end() through to the end of the basic block.  This can cause<br>StackColoring to incorrectly merge %buf with another stack slot.<br><br>Fix by removing the incorrect Starts[pos].isValid() and<br>Finishes[pos].isValid() checks.<br><br>Just doing:<br>      Starts[pos] = Indexes->getMBBStartIdx(MBB);<br>      Finishes[pos] = Indexes->getMBBEndIdx(MBB);<br>unconditionally would be enough to fix the bug, but it causes some<br>test failures due to stack slots not being merged when they were<br>before.  So, in order to keep the existing tests passing, treat LiveIn<br>and LiveOut separately rather than approximating the live ranges by<br>merging LiveIn and LiveOut.<br><br>This fixes PR15707.<br><br></div><span><Mail Attachment></span></div></blockquote></div><br></div></body></html>