<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thanks, Vladimir!<div class=""><br class=""></div><div class="">I did the proposed change.</div><div class="">Here’s the updated webrev: <a href="http://cr.openjdk.java.net/~iveresov/8068881/webrev.02/" class="">http://cr.openjdk.java.net/~iveresov/8068881/webrev.02/</a></div><div class=""><br class=""></div><div class="">Thanks!</div><div class="">igor</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jan 19, 2015, at 11:22 AM, Vladimir Ivanov <<a href="mailto:vladimir.x.ivanov@oracle.com" class="">vladimir.x.ivanov@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">Looks good.</span><br style="font-family: Helvetica; font-size: 12px; 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=""><br style="font-family: Helvetica; font-size: 12px; 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=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">Just a cosmetic suggestion (feel free to ignore it):</span><br style="font-family: Helvetica; font-size: 12px; 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=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">it looks like if (k == n->req() - 1) is better suited for PhaseChaitin::merge_multidefs() than PhaseChaitin::possibly_merge_multidef().</span><br style="font-family: Helvetica; font-size: 12px; 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=""><br style="font-family: Helvetica; font-size: 12px; 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=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">Best regards,</span><br style="font-family: Helvetica; font-size: 12px; 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=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">Vladimir Ivanov</span><br style="font-family: Helvetica; font-size: 12px; 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=""><br style="font-family: Helvetica; font-size: 12px; 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=""><span style="font-family: Helvetica; font-size: 12px; 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; float: none; display: inline !important;" class="">On 1/19/15 9:35 PM, Igor Veresov wrote:</span><br style="font-family: Helvetica; font-size: 12px; 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" style="font-family: Helvetica; font-size: 12px; 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="">I just amended the change with an extended comment on why it’s enough to<br class="">track base registers only and ignore multi-register effects (like<br class="">multi-registers lrg definitions and fat projections), it is subtle, but<br class="">saves unnecessary work:<br class=""><br class="">// We just updated the last edge, now null out the value produced by the<br class="">instruction itself,<br class="">// since we're only interested in defs implicitly defined by the uses.<br class="">We are actually interested<br class="">// in tracking only redefinitions of the multidef lrgs in the same<br class="">register. For that matter it's enough<br class="">// to track changes in the base register only and ignore other effects<br class="">of multi-register lrgs<br class="">// and fat projections. It is also ok to ignore defs coming from<br class="">singledefs. After an implicit<br class="">// overwrite by one of those our register is guaranteed to be used by<br class="">another lrg and we won't<br class="">// attempt to merge it.<br class=""><br class=""><br class="">I also tightened the code following the comment to do updates only for<br class="">multidefs lrgs (postaloc.cpp):<br class=""><br class="">    lrg = _lrg_map.live_range_id(n);<br class="">- if (lrg > 0) {<br class="">+ if (lrg > 0 && lrgs(lrg).is_multidef()) {<br class="">OptoReg::Name reg = lrgs(lrg).reg();<br class=""><a href="http://reg2defuse.at/" class="">reg2defuse.at</a><span class="Apple-converted-space"> </span><<a href="http://reg2defuse.at/" class="">http://reg2defuse.at</a>>(reg).clear();<br class="">    }<br class=""><br class="">Webrev updated in place.<br class=""><br class="">Sorry about the last minute change.<br class="">igor<br class=""><br class=""><blockquote type="cite" class="">On Jan 18, 2015, at 12:25 PM, Igor Veresov <<a href="mailto:igor.veresov@oracle.com" class="">igor.veresov@oracle.com</a><br class=""><<a href="mailto:igor.veresov@oracle.com" class="">mailto:igor.veresov@oracle.com</a>>> wrote:<br class=""><br class="">After register allocation we may end up with nodes in the same block<br class="">using different inputs that are in fact a part of a multidef lrg.<br class="">Since that would confuse the scheduler, the post-allocation copy<br class="">removal attempts to select a one of the inputs and replace all the<br class="">uses that refer to the same value within the block. That works most of<br class="">the time except when we try to replace an input coming from a phi that<br class="">has the only user. In that case the phi goes dead along with the spill<br class="">copy that merges the values, which produces incorrect code.<br class="">Unfortunately there is no way to make a proper selection of a reaching<br class="">def - the easiest counter example is having to select from two phis.<br class=""><br class="">The solution to the problem is to introduce a node that acts like a<br class="">phi but is without control (or rather something like MergeMem) that<br class="">would merge the defs (that are really the same value and are a part of<br class="">a multidef lrg). The following change adds a new node (MachMerge) and<br class="">a pass after the post-allocation copy removal to insert them when<br class="">needed. Even though it’s a separate pass it’s a very fast linear<br class="">traversal.<br class=""><br class="">Webrev: <a href="http://cr.openjdk.java.net/~iveresov/8068881/webrev.01/" class="">http://cr.openjdk.java.net/~iveresov/8068881/webrev.01/</a><br class=""><br class="">Tested with the failing method in weblogic, jprt, CTW<br class=""><br class="">Thanks,<br class="">igor</blockquote></blockquote></div></blockquote></div><br class=""></div></body></html>