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

[Rickard Bäckman]

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
>>>> 
>>