RFR (XS): 8151988: Fix for bug - Hotspot deoptimizes div/mod pair usage

[Deshpande, Vivek R]

Hi Vladimir

I have updated the patch with suggested changes.
http://cr.openjdk.java.net/~vdeshpande/8151988/webrev.03/

I did run the jtreg tests and also tested with the test case provided along with the bug.

Regards,
Vivek

-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Monday, August 22, 2016 11:44 AM
To: Deshpande, Vivek R; hotspot compiler
Cc: Viswanathan, Sandhya
Subject: Re: RFR (XS): 8151988: Fix for bug - Hotspot deoptimizes div/mod pair usage

Thank you for update, Vivek

I would prefer to have duplicated local Node* d = NULL only for Op_ModI and Op_ModL to avoid looking few pages above.
The comment should be changed:

+      // Remove useless control edge in case of not mod-zero.

Did you run tests?

Thanks,
Vladimir

On 8/22/16 11:27 AM, Deshpande, Vivek R wrote:
> Hi Vladimir
>
> Please find the updated patch at this location for your review.
> This patch removes the control edge for ModNode with same condition in Compile::final_graph_reshaping_imp().
> http://cr.openjdk.java.net/~vdeshpande/8151988/webrev.02/
>
> Regards,
> Vivek
>
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Tuesday, August 16, 2016 2:17 PM
> To: Deshpande, Vivek R; hotspot compiler
> Cc: Viswanathan, Sandhya
> Subject: Re: RFR (XS): 8151988: Fix for bug - Hotspot deoptimizes 
> div/mod pair usage
>
> On 8/16/16 2:13 PM, Vladimir Kozlov wrote:
>> On 8/8/16 5:10 PM, Deshpande, Vivek R wrote:
>>> Hi Vladimir
>>>
>>> Commenting out the code you mentioned worked, as the control edge 
>>> for ModINode is never set to NULL.
>>> Shall I send the patch with that change ?
>>
>> Good, thank you for verifying it.
>> ModLNode::Ideal() also has the same problem.
>> I think removing control edge (when != 0) is still useful for 
>> standalone
>
> I mean with the same condition:
> if( in(0) && (ti->_hi < 0 || ti->_lo > 0) ) {
>
> Vladimir
>
>> Mod node but it should be done Compile::final_graph_reshaping_imp()
>> when
>> find_similar() returns false or UseDivMod is false.
>>
>> Thanks,
>> Vladimir
>>
>>>
>>> Regards,
>>> Vivek
>>>
>>> -----Original Message-----
>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>> Sent: Wednesday, August 03, 2016 4:10 PM
>>> To: Deshpande, Vivek R <vivek.r.deshpande at intel.com>; hotspot 
>>> compiler <hotspot-compiler-dev at openjdk.java.net>
>>> Subject: Re: RFR (XS): 8151988: Fix for bug - Hotspot deoptimizes 
>>> div/mod pair usage
>>>
>>> Hi Vivek,
>>>
>>> The problem is not that DivModINode does not have control edge set 
>>> but that Div and Mod nodes have different setting and have to be 
>>> ordered according their uses and 0 checks of their arguments.
>>>
>>> Please, find where ModI node's control edge is replaced with NULL.
>>> May be next in ModINode::Ideal()
>>>
>>>     // Check for useless control input
>>>     // Check for excluding mod-zero case
>>>     if( in(0) && (ti->_hi < 0 || ti->_lo > 0) ) {
>>>       set_req(0, NULL);        // Yank control input
>>>       return this;
>>>     }
>>>
>>> Try to comment it.
>>>
>>> Thanks,
>>> Vladimir
>>>
>>> On 8/1/16 11:49 AM, Deshpande, Vivek R wrote:
>>>> Hi Vladimir
>>>>
>>>> I have fixed the problem of null control edge getting generated for 
>>>> DivMod node in this later patch.
>>>> Could you please take a look at the webrev at this location:
>>>> http://cr.openjdk.java.net/~vdeshpande/8151988/webrev.01/
>>>>
>>>> Regards,
>>>> Vivek
>>>>
>>>> -----Original Message-----
>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>> Sent: Wednesday, July 27, 2016 7:54 PM
>>>> To: Deshpande, Vivek R; hotspot compiler
>>>> Cc: Viswanathan, Sandhya
>>>> Subject: Re: RFR (XS): 8151988: Fix for bug - Hotspot deoptimizes 
>>>> div/mod pair usage
>>>>
>>>> If control edge is different (one of them not NULL) I don't think 
>>>> we can replace Div and Mod nodes.
>>>> Control edge enforce the order of execution of data nodes. DivModI 
>>>> node takes control from ModI node which can put DivModI below users 
>>>> of DivI node. So you will have shceduler problem.
>>>>
>>>> So why control edge is NULL for DivI (or ModI) node? Control edge 
>>>> should come from zero_check_int() which is called for ModI and DivI 
>>>> nodes.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 7/27/16 5:56 PM, dean.long at oracle.com wrote:
>>>>> How about this?  I think it gives the same result, but with fewer 
>>>>> tests
>>>>>
>>>>>                 if (use->in(j) != in(j) &&(j != 0 || (use->in(j) 
>>>>> != NULL && in(j) != NULL)) {
>>>>>                   break;
>>>>>                 }
>>>>>
>>>>>
>>>>> dl
>>>>>
>>>>> On 7/27/16 4:14 PM, Deshpande, Vivek R wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> I have resolved the bug and fix for the same is here.
>>>>>>
>>>>>> http://cr.openjdk.java.net/~vdeshpande/8151988/webrev.00/
>>>>>>
>>>>>> I have also updated the JBS entry.
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8151988
>>>>>>
>>>>>> Would you please review it.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> Vivek
>>>>>>
>>>>>