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

[Vladimir Kozlov]

This looks good.

thanks,
Vladimir

On 5/1/14 11:55 PM, Tobias Hartmann wrote:
> 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
>>>
>