JDK-8035283 : Second phase of branch shortening doesn't account for loop alignment
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • CPU: sparc
  • Submitted: 2014-02-19
  • Updated: 2016-05-02
  • Resolved: 2014-02-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 7 JDK 8 JDK 9
7u51Fixed 8u11Fixed 9 b04Fixed
Related Reports
Relates :  
JDK-8155801 - solaris sparc jdk 8u45: guarantee((int)(blk_starts[i+1] - blk_starts[i]) >= (current_offset - blk_offset)) shouldn't increase block size
Relates :  
JDK-7063629 - use cbcond in C2 generated code on T4
Relates :  
JDK-8056124 - Hotspot should use PICL interface to get cacheline size on SPARC
Description
On SPARC with cbcond we can fail with:
# Internal Error (output.cpp:1576), pid=3601, tid=15 
# guarantee((int)(blk_starts[i+1] - blk_starts[i]) >= (current_offset - blk_offset)) failed: shouldn't increase block size 

The customer hit the bug with the 32bit version on a virtual machine running on T-series. We didn't identify that machine as niagara so we used the default loop alignment (16 bytes instead of 4). I cannot reproduce the crash on 64bit, but the problem exists there as well.

It very hard to write a unit test for the fix (at least I wasn't able to), since the code must specifically trigger block rotation in loop to make sure that the loop header starts with a compare.
Comments
Critical request motivation: This fix was delivered to the Solaris team as a BPR. If we can include it in the upcoming CPU, it will make it easier for the Solaris team to update, and we won't need to provide a second BPR
06-03-2014
Based on the recent VM 8u20 nightly test run restuls SQE OK to take the fix into CPU14_02
06-03-2014
Suggested fix: diff --git a/src/share/vm/opto/output.cpp b/src/share/vm/opto/output.cpp --- a/src/share/vm/opto/output.cpp +++ b/src/share/vm/opto/output.cpp @@ -499,9 +499,19 @@ if (bnum > i) { // adjust following block's offset offset -= adjust_block_start; } + + // This block can be a loop header, account for the padding + // in the previous block. + int prev_block_loop_pad = b->code_alignment() - relocInfo::addr_unit(); + if (i > 0 && prev_block_loop_pad > 0) { + assert(br_offs >= prev_block_loop_pad, "Should have at least a padding on top"); + } else { + // First block or not a loop + prev_block_loop_pad = 0; + } // In the following code a nop could be inserted before // the branch which will increase the backward distance. - bool needs_padding = ((uint)br_offs == last_may_be_short_branch_adr); + bool needs_padding = ((uint)(br_offs - prev_block_loop_pad) == last_may_be_short_branch_adr); if (needs_padding && offset <= 0) offset -= nop_size;
19-02-2014