United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-8035283 : Second phase of branch shortening doesn't account for loop alignment

Details
Type:
Bug
Submit Date:
2014-02-19
Status:
Resolved
Updated Date:
2014-09-04
Project Name:
JDK
Resolved Date:
2014-02-27
Component:
hotspot
OS:
Sub-Component:
compiler
CPU:
sparc
Priority:
P3
Resolution:
Fixed
Affected Versions:
9
Fixed Versions:

Related Reports
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Backport:
Relates:
Relates:

Sub Tasks

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
URL:   http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/b3e1a903b6e8
User:  lana
Date:  2014-03-11 02:12:48 +0000

                                     
2014-03-11
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;

                                     
2014-02-19
URL:   http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/b3e1a903b6e8
User:  kvn
Date:  2014-02-27 02:03:37 +0000

                                     
2014-02-27
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
                                     
2014-03-06
Based on the recent VM 8u20 nightly test run restuls SQE OK to take the fix into CPU14_02
                                     
2014-03-06



Hardware and Software, Engineered to Work Together