RFR: 8151661: Performance regression on Solaris-SPARC in 9-b103

[Andrew Haley]

On 08/06/16 08:14, Rahul Raghavan wrote:

> 4. http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-December/020450.html

> From this old 8145438 - RFR email thread, seems the adlc change was
> done independently, not as an integral part of original
> 8145438/8144028 -fix.

> The same change was done based on the assumption that short and long
> branch variants cannot have different predicates.

> But this is wrong for above SPARC cases.

> Reverting only the adlc change, could not find any issues with -
>     JPRT testing (default -testset hotspot) , 'hotspot/test/compiler/codegen/8144028/BitTests.java' jtreg test run for Sparc, AArch64)
> 
> So in general predicates for short and long branches could be
> different and should not be compared for check_branch_variant()!

> The above proposed webrev.00 fix proposal is the same reverting
> earlier 8145438 adlc change.

That just re-introduces the bug.  check_branch_variant() is suposed to
test whether one branch can be replaced by another, and the predicate
may be an important part of that.  For example, in the case of AArch64
some predicates test if an operand is a power of two: only then may a
bit test and branch instruction be used.  The versions of branch with
and without this test are not semantically equivalent, so one branch
cannot be replaced by another.

I wonder if there is a better way to solve this problem without
weakening check_branch_variant().  The fact that it didn't check
predicates to see if one branch can be replaced by another was a bug.
It was a useful bug, sure, but still a bug.

We could -- in extremis -- #ifdef AARCH64 in check_branch_variant(),
but it would be better to find another way to do it.  For example, we
could define a semantic_predicate and use that.  That way we would
separate predicates which change the meaning of patterns from those
which merely enable them based on some global flag.

Andrew.