<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Aug 2, 2009, at 11:53 PM, Jakob Stoklund Olesen wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><br>On 03/08/2009, at 07.42, Evan Cheng wrote:<br><br><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">But this part is OK, right? <imp-def> can redefine a live register.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">It shouldn't. Are you talking about re-defining part of a larger<br></blockquote><blockquote type="cite">register? In that case, it should first kill the larger register and<br></blockquote><blockquote type="cite">then redefine it. At least that's what livevariable does. It's unclear<br></blockquote><blockquote type="cite">if it's realistic to enforce this in register scavenger. This might be<br></blockquote><blockquote type="cite">too difficult to maintain. I need to see an example.<br></blockquote><br>At least that makes the rules clearer - no special treatment for  <br>implicit operands. This means that you must always know if a register  <br>is dead or alive before use use of define it in any way. It could work.<br></div></blockquote><div><br></div>Right.</div><div><br><blockquote type="cite"><div><br>I don't think the scavenger should enforce this rule through  <br>assertions. A double define does not break the scavenger in any way,  <br>it just makes it a bit less efficient - it is a missed available  <br>register, that is all. I think the machine code verifier should catch  <br>these bugs, and the scavenger should only assert when stuff would break.<br></div></blockquote><div><br></div>You are absolutely right. But we're not yet running the machine verifier by default that means we are using the scavenger for that purpose. It caught a lot of corner cases like these. On more than one occasion I'd thought about just removing the assertions. :-) Note the assertions are turned off in release mode so these bugs are being left through.</div><div><br><blockquote type="cite"><div><br>Use of undefined registers is another matter. It means that a register  <br>was killed too soon, and the scavenger could mistakenly overwrite it.  <br>That would be bad, and an assert is in order.<br></div></blockquote><div><br></div>Right. We ought to use the newly introduced error reporting mechanism instead. Assertions are not strong enough here. The only concern I have is the compile time cost. Some of the assertions checks are implemented less than optimally.</div><div><br><blockquote type="cite"><div><br>I will implement the strict rules in the verifier. Then we can see how  <br>bad the status quo is and get some examples of breakage.<br></div></blockquote><div><br></div>Ok thanks. This is really really hard to get right. I really appreciate that someone else is hacking on it. :-)</div><div><br></div><div> Please see my replies to</div><div><p style="margin: 0.0px 0.0px 1.0px 64.0px; text-indent: -64.0px; font: 12.0px Helvetica"><b>[PATCH] Fix Bug 4657: register scavenger<span class="Apple-tab-span" style="white-space:pre">   </span>asserts with subreg lowering</b></p><div>and</div><div><p style="margin: 0.0px 0.0px 1.0px 64.0px; text-indent: -64.0px; font: 12.0px Helvetica"><b> [PATCH] Accidental <kill> on two-address operand</b></p><div>for more discussion on these issues.</div><div><br></div><div>Evan</div></div><div><br></div><blockquote type="cite"><div><br>/jakob<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></div></blockquote></div><br></body></html>