<html><body bgcolor="#FFFFFF"><div>Hi Chris,</div><div><br></div><div>Thanks for looking at it. Could you sponsor this change, please?<br><br><div>Thanks,</div></div><div>Kris</div><div><br>On 2012-5-30, at 8:16, Christian Thalinger <<a href="mailto:christian.thalinger@oracle.com">christian.thalinger@oracle.com</a>> wrote:<br><br></div><div></div><blockquote type="cite"><div>Looks good. Thanks for fixing this. It hit me quite often while working on C1 and I never had time to fix it.<div><br></div><div>-- Chris</div><div><br><div><div>On May 29, 2012, at 12:10 PM, Krystal Mok wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Hi all,</div><div><br></div><div>Could I have a couple of review for this patch, please:</div><div><a href="https://gist.github.com/2829978#file_c1_fix_printable_bci.patch"><a href="https://gist.github.com/2829978#file_c1_fix_printable_bci.patch">https://gist.github.com/2829978#file_c1_fix_printable_bci.patch</a></a></div>
<div><br></div><div>I hit the "assert(has_printable_bci()) failed: _printable_bci should have been</div><div>set" assertion when I was doing some experiment on C1, and needed to use</div><div>-XX:+PrintCanonicalization for verification. It turns out that this flag is</div>
<div>pretty broken, that quite a few places didn't set printable_bci appropriately.</div><div><br></div><div>This patch tries to fix the missing printable_bcis.</div><div>The link also includes a simple example before and after applying this patch.</div>
<div><br></div><div>c1_Instructions.hpp:</div><div>Added set_printable_bci() to Local and Phi's constructor.</div><div>A "Local" instruction models an incoming argument, which gets its value before</div><div>
method entry, so I'm setting all Local's printable_bci to -1.</div><div>A reasonable printable_bci for a "Phi" instruction is the same as the start bci</div><div>of the basic block to which it belongs. I had to use a weird cast to get rid of</div>
<div>a "invalid use of incomplete type 'struct BlockBegin' error from GCC.</div><div><br></div><div>c1_Canonicalizer.cpp:</div><div>Added a default x->set_printable_bci(bci()) to Canonicalizer::set_canonical().</div>
<div>There are quite a few place in Canonicalizer that doesn't specify the</div><div>printable_bci for the newly created substitution instruction. It's reasonable</div><div>to just set that to the "current" bci, which is the bci of the instruction to</div>
<div>be substituted.</div><div>Also adjusted the order of a set_bci() call in Canonicalizer::do_If, so that</div><div>the new code above could pick up the modified bci in set_canonical().</div><div><br></div><div>Regards,</div>
<div>Kris</div>
</blockquote></div><br></div></div></blockquote></body></html>