RFR: 8211100: hotspot C1 issue with comparing long numbers on x86 32-bit

[Igor Veresov]

Edit: It may be more consistent to check for is_double_cpu() instead of T_LONG. Although that’s semantically equivalent.

> On Sep 26, 2018, at 9:35 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
> 
> It doesn’t seem to me like the proper way to fix it. The problem is that the cmp is destroying opr1 without telling the register allocator about it.
> 
> One possible solution would be to make opr1 also a temp (see LIR_OpVisitState::visit(LIR_Op* op) in c1_LIR.cpp), only for x86 32bit and only if the operand type is T_LONG. 
> Another solution is to maintain a temporary register for lir_cmp and use it to save/restore opr1 when emitting the code in LIR_Assembler::comp_op(). Again, the temporary register has to be there only for x86 32bit and T_LONG.
> 
> igor
> 
> 
>> On Sep 26, 2018, at 1:29 AM, Tobias Hartmann <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>> 
>> Hi Dmitry,
>> 
>> this looks good to me but Igor (who implemented 8201447) should have a look as well.
>> 
>> Best regards,
>> Tobias
>> 
>> On 26.09.2018 09:04, Dmitry Cherepanov wrote:
>>> Hi Tobias,
>>> 
>>> Thanks for the review, updated patch avoids the additional move on x86_64 and includes the
>>> regression test.
>>> 
>>> http://cr.openjdk.java.net/~dcherepanov/8211100/webrev.01/ <http://cr.openjdk.java.net/~dcherepanov/8211100/webrev.01/>
>>> <http://cr.openjdk.java.net/%7Edcherepanov/8211100/webrev.01/ <http://cr.openjdk.java.net/%7Edcherepanov/8211100/webrev.01/>>
>>> 
>>> Dmitry
>>> 
>>>> On Sep 25, 2018, at 6:40 PM, Tobias Hartmann <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>
>>>> <mailto:tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>>> wrote:
>>>> 
>>>> Hi Dmitry,
>>>> 
>>>> Shouldn't this at least be guarded by an #ifndef _LP64 to avoid the additional move on x86_64?
>>>> 
>>>> Could you please add the regression test to the webrev? Or did this reproduce with other tests?
>>>> 
>>>> Thanks,
>>>> Tobias
>>>> 
>>>> On 25.09.2018 16:00, Dmitry Cherepanov wrote:
>>>>> Hello,
>>>>> 
>>>>> Please review a patch that resolves issue in x86 32bit builds. It slightly adjusts the fix for
>>>>> JDK-8201447 (C1 does backedge profiling incorrectly) by creating a copy of the left operand and
>>>>> using it for incrementing backedge counter.
>>>>> 
>>>>> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8211100 <https://bugs.openjdk.java.net/browse/JDK-8211100>
>>>>> webrev: http://cr.openjdk.java.net/~dcherepanov/8211100/webrev.00/ <http://cr.openjdk.java.net/~dcherepanov/8211100/webrev.00/>
>>>>> <http://cr.openjdk.java.net/%7Edcherepanov/8211100/webrev.00/ <http://cr.openjdk.java.net/%7Edcherepanov/8211100/webrev.00/>>
>>>>> 
>>>>> Thanks,
>>>>> 
>>>>> Dmitry
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20180926/f1f22805/attachment-0001.html>