RFR(M) 8024924: Intrinsify java.lang.Math.addExact

[Vladimir Kozlov]

It is good. Ask Christian and Igor to get second review.

Thanks,
Vladimir

On 9/20/13 4:34 AM, Rickard Bäckman wrote:
> Here is an updated webrev:
>
> http://cr.openjdk.java.net/~rbackman/8024924.2/
>
> Moved the tests to compiler/intrinsics/mathexact
> and moved the new nodes to mathexactnode.[ch]pp
>
> Thanks
> /R
>
> On Sep 19, 2013, at 9:36 PM, Rickard Bäckman wrote:
>
>> Vladimir,
>>
>> see inline.
>>
>> On Sep 19, 2013, at 9:29 PM, Vladimir Kozlov wrote:
>>
>>> On 9/19/13 11:46 AM, Rickard Bäckman wrote:
>>>> Vladimir,
>>>>
>>>> thanks for the review. I'll update the code accordingly.
>>>>
>>>> On Sep 19, 2013, at 8:15 PM, Vladimir Kozlov wrote:
>>>>
>>>>> Rickard,
>>>>>
>>>>> You need to provide more description for public and include the link to the bug report which now is accessible from outside:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8024924
>>>>
>>>> Sorry missed that one.
>>>>
>>>>>
>>>>> When result of MathExact is not used we should replace it with top to remove uncommon trap.
>>>>
>>>> You are saying that if the ProjNode (result) doesn't have an out we should replace the MathExact with top() right?
>>>
>>> Yes, if it is IGVN. Make sure FlagsProjNode handles such case (top input).
>>>
>>> ProjNode could be gone too, eliminated by remove_globally_dead_node(). You may need to add MathExact node check to has_special_unique_user() so MathExact will be put on _worklist in such case:
>>>
>>>            } else if (in->outcnt() == 1 &&
>>>                       in->has_special_unique_user()) {
>>>              _worklist.push(in->unique_out());
>>>
>>> Note, usually we return top from ::Value() method.
>>
>> But can I really remove the uncommon trap if the use of the value goes dead?
>> I mean:
>>
>> addExact(big value, big value)
>>
>> that doesn't have a use of the sum. But it would throw an exception.
>> If we remove the uncommon trap… we would no longer throw an exception.
>>
>> /R
>>
>>>
>>>>>
>>>>> I think it would be better to create new mathexactnode.?pp files for new mathexact nodes. Our *node.?pp files become too fat.
>>>>
>>>> Good idea.
>>>>
>>>>>
>>>>> In MultiNode::proj_out(proj) check is_FlagsProj() first since is_Proj() is also true for it (since it is subclass).
>>>>
>>>> No it doesn't. is_Proj() is controlled by the macros in nodes.hpp. FlagsProjNode is a child of CmpNode there and can't be a member of both. I hit the assert in proj_out()
>>>
>>> You are right.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>>
>>>>>
>>>>> Next test file misses Copyright header:
>>>>>
>>>>> test/compiler/8024924/Verify.java
>>>>
>>>> Ok.
>>>>
>>>>>
>>>>> We are moving to creation of subcomponent subdirs for new tests. Could you move that into test/compiler/intrinsic/8024924 ?
>>>>
>>>> Ok.
>>>>
>>>> /R
>>>>
>>>>>
>>>>> thanks,
>>>>> Vladimir
>>>>>
>>>>> On 9/18/13 11:55 AM, Rickard Bäckman wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> can I please have reviews for this change.
>>>>>> Jump on overflow is implemented on x86 32&64-bit.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~rbackman/8024924.1/
>>>>>>
>>>>>> Thanks
>>>>>> /R
>>>>>>
>>>>
>>
>