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

[Igor Veresov]

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> 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/92b3654e/attachment.html>