http://cr.openjdk.java.net/~kvn/7039652/webrev

Fixed 7039652: Performance regression after 7004547 changes

In the test case initial stride is -8 so the loop is unrolled only once after
7004547 changes moved next condition to the beginning of policy_unroll() method:

// Check for stride being a small enough constant
if (abs(cl->stride_con()) > (1<<3)) return false;

Before 7004547 changes that check was done at the end of policy_unroll() and the
method returned 'true' if there were a lot 'xor' nodes in the loop regardless
the stride check. As result the loop in the test case was unrolled twice.

The stride check serves additional purpose to limit unrolling to 16 (when
initial stride is '1'). But as the test shows it may prevent legit unroll with
bigger stride.

The fix is to use unrolled_count() to limit unrolling and use the stride check
only for initial stride value. I don't understand why there is such limit on
stride but it is for an other investigation (RFE).

I ran refworkload and don't see any affects from this fix.

Search Discussions

  • Tom Rodriguez at Apr 28, 2011 at 12:34 pm
    Seems ok.

    tom
    On Apr 27, 2011, at 4:17 PM, Vladimir Kozlov wrote:

    http://cr.openjdk.java.net/~kvn/7039652/webrev

    Fixed 7039652: Performance regression after 7004547 changes

    In the test case initial stride is -8 so the loop is unrolled only once after 7004547 changes moved next condition to the beginning of policy_unroll() method:

    // Check for stride being a small enough constant
    if (abs(cl->stride_con()) > (1<<3)) return false;

    Before 7004547 changes that check was done at the end of policy_unroll() and the method returned 'true' if there were a lot 'xor' nodes in the loop regardless the stride check. As result the loop in the test case was unrolled twice.

    The stride check serves additional purpose to limit unrolling to 16 (when initial stride is '1'). But as the test shows it may prevent legit unroll with bigger stride.

    The fix is to use unrolled_count() to limit unrolling and use the stride check only for initial stride value. I don't understand why there is such limit on stride but it is for an other investigation (RFE).

    I ran refworkload and don't see any affects from this fix.
  • Vladimir Kozlov at Apr 28, 2011 at 2:32 pm
    Thank you, Tom

    Vladimir

    Tom Rodriguez wrote:
    Seems ok.

    tom
    On Apr 27, 2011, at 4:17 PM, Vladimir Kozlov wrote:

    http://cr.openjdk.java.net/~kvn/7039652/webrev

    Fixed 7039652: Performance regression after 7004547 changes

    In the test case initial stride is -8 so the loop is unrolled only once after 7004547 changes moved next condition to the beginning of policy_unroll() method:

    // Check for stride being a small enough constant
    if (abs(cl->stride_con()) > (1<<3)) return false;

    Before 7004547 changes that check was done at the end of policy_unroll() and the method returned 'true' if there were a lot 'xor' nodes in the loop regardless the stride check. As result the loop in the test case was unrolled twice.

    The stride check serves additional purpose to limit unrolling to 16 (when initial stride is '1'). But as the test shows it may prevent legit unroll with bigger stride.

    The fix is to use unrolled_count() to limit unrolling and use the stride check only for initial stride value. I don't understand why there is such limit on stride but it is for an other investigation (RFE).

    I ran refworkload and don't see any affects from this fix.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouphotspot-compiler-dev @
categoriesopenjdk
postedApr 27, '11 at 4:17p
activeApr 28, '11 at 2:32p
posts3
users2
websiteopenjdk.java.net

2 users in discussion

Vladimir Kozlov: 2 posts Tom Rodriguez: 1 post

People

Translate

site design / logo © 2021 Grokbase