RFR(M) 8068881: SIGBUS in C2 compiled method weblogic.wsee.jaxws.framework.jaxrpc.EnvironmentFactory$SimulatedWsdlDefinitions.<init>

[Igor Veresov]

Thanks, Vladimir!

I did the proposed change.
Here’s the updated webrev: http://cr.openjdk.java.net/~iveresov/8068881/webrev.02/ <http://cr.openjdk.java.net/~iveresov/8068881/webrev.02/>

Thanks!
igor

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20150119/724fc733/attachment-0001.html>