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

[Dmitry Cherepanov]

Hi Igor,

Thanks for the suggestions. I tried to make the opr1 a temporary

http://cr.openjdk.java.net/~dcherepanov/8211100/webrev.02/

but the generated code still has the problem. Looking into the log with -XX:TraceLinearScanLevel=4 (http://cr.openjdk.java.net/~dcherepanov/8211100/TraceLinearScanLevel.02.log) seems like the reason for this is that the opr1 (virtual register R165 in the log) is also an input operand and its range becomes wider and the shorter ranges (corresponding to the opr1 marked as temp) are merged to the single range. Can the input operand be temporary at the same time?

Dmitry

On Sep 27, 2018, at 2:18 AM, Igor Veresov <igor.veresov at oracle.com<mailto:igor.veresov at oracle.com>> wrote:

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<mailto: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/%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>> 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
webrev: http://cr.openjdk.java.net/~dcherepanov/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/20181001/08e21898/attachment-0001.html>