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

Vladimir Kozlov vladimir.kozlov at oracle.com
Mon Feb 24 14:55:52 PST 2014

Previous message: RFR(S): 8035283 Second phase of branch shortening doesn't account for loop alignment
Next message: RFR(S): 8035283 Second phase of branch shortening doesn't account for loop alignment
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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

448 int max_loop_pad = nb->code_alignment()-relocInfo::addr_unit();

produces the same value as new code at the line #505:

505 int prev_block_loop_pad = block->code_alignment() - relocInfo::addr_unit();

because code_alignment() is not one-liner.

I am fine with adding additional array block_worst_case_pad[]:

    int max_loop_pad = nb->code_alignment()-relocInfo::addr_unit();

 block_worst_case_pad[i+1] = max_loop_pad;
  if (max_loop_pad > 0) {

Also prev_block_loop_pad 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 block_padding = block_worst_case_pad[i];

```
    if (i > 0 && block_padding > 0) {
```

      assert(br_offs >= block_padding, "Should have at least a

padding on top");

```
    } else {
```
```
      // First block or not a loop
```
```
      block_padding = 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

Previous message: RFR(S): 8035283 Second phase of branch shortening doesn't account for loop alignment
Next message: RFR(S): 8035283 Second phase of branch shortening doesn't account for loop alignment
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the hotspot-compiler-dev mailing list