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:57:20 PST 2014

Vladimir, that for the suggestion.

Here the new version of the change: http://cr.openjdk.java.net/~iveresov/8035283/webrev.02/

Thanks! igor

On Feb 24, 2014, at 2:55 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

After discussing with John I agree with him. From current code it is not obvious that code at the line #448:

448 int maxlooppad = nb->codealignment()-relocInfo::addrunit(); produces the same value as new code at the line #505: 505 int prevblocklooppad = block->codealignment() - relocInfo::addrunit(); because codealignment() is not one-liner. I am fine with adding additional array blockworstcasepad[]: int maxlooppad = nb->codealignment()-relocInfo::addrunit(); + blockworstcasepad[i+1] = maxlooppad; if (maxlooppad > 0) { Also prevblocklooppad name in new code is confusing. It is padding for current block even so the padding is inserted in previous block. + // This block may need special alignment, account for + // the padding before it. + int blockpadding = blockworstcasepad[i]; + if (i > 0 && blockpadding > 0) { + assert(broffs >= blockpadding, "Should have at least a padding on top"); + } else { + // First block or not a loop + blockpadding = 0; + } Thanks, Vladimir On 2/24/14 12:27 PM, John Rose wrote: On Feb 23, 2014, at 11:02 PM, Igor Veresov <igor.veresov at oracle.com_ _<mailto: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 '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! — John

More information about the hotspot-compiler-dev mailing list