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

[Tobias Hartmann]

Thank you Vladimir.

Best,
Tobias

On 02.05.2014 19:31, Vladimir Kozlov wrote:
> 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
>>>>
>>