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

[Vladimir Kozlov]

Thank you, Vivek

Changes look good. I ran tests on my side too - results are good.
I will sponsor the fix.

Thanks,
Vladimir

On 8/23/16 10:03 AM, Deshpande, Vivek R wrote:
> 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
>>>>>>>
>>>>>>