RFR(S): 8035283 Second phase of branch shortening doesn't account for loop alignment

[John Rose]

On Feb 23, 2014, at 11:02 PM, Igor Veresov <igor.veresov at oracle.com> wrote:

> May I please have a second review of this?
> Webrev: http://cr.openjdk.java.net/~iveresov/8035283/webrev.01/

I don't understand the force of the assert; it seems to be true mostly by accident.

Maybe you want an assert that 'last_may_be_short_branch_adr' does not fall between (br_offs - prev_block_loop_pad)+1 and br_offs, inclusive?

It took me a long time to convince myself that moving the goalpost for the comparison to 'last_may_be_short_branch_adr' was safe.  Really, the argument hinges on the fact that all layout info. is relative to a pessimistic assumption that the maximum possible padding (block->code_alignment() - relocInfo::addr_unit()) is always inserted.

I suggest making the linkage to that assumption clearer, by hoisting the crucial expression 'block->code_alignment() - relocInfo::addr_unit()' as follows:

  uint*      worst_case_pad  = NEW_RESOURCE_ARRAY(uint,nblocks);
...

  worst_case_pad[i] = block->code_alignment() - relocInfo::addr_unit();

Then use the array reference directly instead of the now-linked uses of code_alignment etc.

This is delicate code!

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140224/1488b637/attachment.html