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

Igor Veresov igor.veresov at oracle.com
Tue Feb 25 09:55:55 PST 2014

John, thanks for the review!

On Feb 24, 2014, at 12:27 PM, John Rose <john.r.rose at oracle.com> wrote:

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.

I assume you’re talking about the first one. The goal of that assert is to basically ensure that br_offs - prev_block_loop_pad doesn’t underflow, which should be true for all the blocks except the first that require padding.

Maybe you want an assert that 'lastmaybeshortbranchadr' does not fall between (broffs - prevblocklooppad)+1 and broffs, inclusive?

It took me a long time to convince myself that moving the goalpost for the comparison to 'lastmaybeshortbranchadr' 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->codealignment() - relocInfo::addrunit()) is always inserted. I suggest making the linkage to that assumption clearer, by hoisting the crucial expression 'block->codealignment() - relocInfo::addrunit()' as follows: uint* worstcasepad = NEWRESOURCEARRAY(uint,nblocks); ... worstcasepad[i] = block->codealignment() - relocInfo::addrunit(); Then use the array reference directly instead of the now-linked uses of codealignment etc. This is delicate code!

I agree, that’ll make the code more robust. I’ll make the changes.

igor

— John

-------------- next part -------------- An HTML attachment was scrubbed... URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140225/86efe5c5/attachment.html

More information about the hotspot-compiler-dev mailing list