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

[Tobias Hartmann]

Thanks, Niclas.

Tobias

On 05/02/2014 12:58 PM, Niclas Adlertz wrote:
> That looks good to me.
>
> Kind Regards,
> Niclas Adlertz
>
> On 05/02/2014 08:55 AM, 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
>>>>
>>
>>