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

[Deshpande, Vivek R]

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