[9] RFR(XS): 8042052: assert(t != NULL) failed: must set before get

[Tobias Hartmann]

Hi Niclas,

thanks for the feedback. Please see comments inline.

On 04/30/2014 05:20 PM, Niclas Adlertz wrote:
> Hi Tobias,
>
> Thank you for catching this.
> I would say you can then change "_gvn.transform(result)" to just 
> "result" inside "inline_pow()", after the call to "finish_pow_exp()" 
> at line 1931:
>
> From:
> // the result from finish_pow_exp is now input to the phi node
>
>
> phi_node->init_req(2, _gvn.transform(result));
>
> To:
> phi_node->init_req(2, result);
>
> since "result" has already been returned from "_gvn.transform()" 
> either inside "finish_pow_exp" or before the call, inside "inline_pow()".

Done.

>
> Also since "_gvn.transform()" possibly returns a new node, this:
> // Make sure type of PhiNode is recorded
> _gvn.transform(result_val);
> return result_val;
>
> should be replaced with:
> return _gvn.transform(result_val);
>
> (I don't think the comment is nescessary since we need to record nodes 
> at many other places as well)

Yes, I missed that _gvn.transform(...) may return a new node. I changed 
the code according to your suggestions.

New webrev: http://cr.openjdk.java.net/~anoll/8042052/webrev.01/ 
<http://cr.openjdk.java.net/%7Eanoll/8042052/webrev.01/>

Thanks,
Tobias


>
> Kind Regards,
> Niclas Adlertz
>
> On 04/30/2014 04:30 PM, Tobias Hartmann wrote:
>> Hi,
>>
>> please review the following patch for bug 8042052.
>>
>> *Problem*
>> The changes introduced by the fix for 8041912 [1] removed the call to
>> set_result(...) in LibraryCallKit::finish_pow_exp(...) that called
>> gvn.transform on the PhiNode results_val. The type of the PhiNode is
>> therefore not recorded in the gvn. The PhiNode is returned and later
>> accessed as an input to a StoreNode in
>> PhaseGVN::transform_no_reclaim(...). The call to StoreNode::Value tries
>> to look up the type of the PhiNode and triggers the assert in
>> PhaseTransform::type(...) because the type is not set.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8042052
>>
>> *Solution*
>> A call to _gvn.transform is added to make sure the type of the PhiNode
>> is recorded.
>>
>> Webrev: http://cr.openjdk.java.net/~anoll/8042052/webrev.00/
>>
>> *Tests:*
>> Failing test (testMathExpDouble), JPRT
>>
>> Thanks,
>> Tobias
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8041912
>>

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