[aarch64-port-dev ] RFR(M): 8145336: PPC64: fix string intrinsics after CompactStrings change

[Doerr, Martin]

Hi Volker,

thank you very much for adapting the non-CompactStrings version of the intrinsics. I especially like that you changed shared code to improve matching of special cases.

Here are some minor change requests:
- I guess you will have to adapt Copyright messages.
- There's a typo in the new comment in library_call: "optimzed".
- The comment for the instruction count (used for loop alignment) is wrong in MacroAssembler::string_indexof_1 (should start with 3 instead of 2).

I have more change requests regarding ppc.ad:

The computation of chr is incorrect for little endian in string_indexOf_imm1_char and string_indexOf_imm1.

Some numbers for compute_padding should be adapted:
int string_indexOf_imm1_charNode::compute_padding(int current_offset) const { return (3*4-current_offset)&31; } 
int string_indexOfCharNode::compute_padding(int current_offset) const { return (3*4-current_offset)&31; } 
int string_compareNode::compute_padding(int current_offset) const { return (2*4-current_offset)&31; }

Some kill effects are missing:
- ctr in all string_indexOf nodes
- cr0, cr1 in string_indexOf_imm1, string_indexOfChar

The new comment for string_indexOfChar claims "// Kill ... needle" which is not true.

Thanks,
 Martin


-----Original Message-----
From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Volker Simonis
Sent: Dienstag, 19. Januar 2016 19:57
To: hotspot compiler <hotspot-compiler-dev at openjdk.java.net>
Cc: ppc-aix-port-dev at openjdk.java.net; aarch64-port-dev at openjdk.java.net
Subject: RFR(M): 8145336: PPC64: fix string intrinsics after CompactStrings change

Hi,

can somebody please review and sponsor this change.

Despite the bug summary, I still had to do some small shared changes
to make this work, so unfortunately I can not push this on my own.

The change also affects aarch64 (although it is minimal and I don't
expect it to break anything) so I cc-ed aarch64-port-dev.

http://cr.openjdk.java.net/~simonis/webrevs/2016/8145336/
https://bugs.openjdk.java.net/browse/JDK-8145336

As described in the bug, this change only fixes the string intrinsics
for the -XX:-UseCompactStrings mode which is still the default on
ppc64. Additionally, support for the new StrIndexOfChar intrinsic was
added because we already had a similar intrinsic for constant string
needles of length one anyway. A later change (which we're already
working on) will add the intrinsics which can handle compact strings.

The current intrinsics can handle both, the new byte-array based
string representation as well as the old char-array based string
representation because we internally still use the new hotspot with
older versions of the class libraries.

I've also ported some of our internal string tests into a new
regression test (TestStringIntrinsics2.java) because the existing
tests didn't exercise all of our intrinsics.

Following the shared changes I had to do:

Until now, UseSSE42Intrinsics was a global shared option which was
used to control the availability of the stringIndexOf intrinsics. But
UseSSE42Intrinsics is actually a x86-specific feature so it doesn't
make a lot of sense to define it for other architectures. I've
therefore moved the flag to globals_x86.hpp and changed the condition
which checks for the ability of the stringIndexOf intrinsics from:

if (!Matcher::has_match_rule(Op_StrIndexOf) || !UseSSE42Intrinsics) {

to:

if (!Matcher::match_rule_supported(Op_StrIndexOf)) {

The Matcher::match_rule_supported() method already calls
Matcher::has_match_rule() anyway. And it is implemented in the .ad
file so I've moved the check for UseSSE42Intrinsics into x86.ad. Other
platforms can now decide in their .ad file if they unconditionally
support the intrinsic or if they need a special feature check. This
change was already briefly discussed in [1].

The other shared change I had to make was in
LibraryCallKit::make_string_method_node() for the "Op_StrEquals" case.
We have optimized intrinsics for the case that one of the strings to
compare is constant, but the  StrEqualsNode is constructed without
taking into account that one of the string length values could be a
constant. This prevented our optimized instruction from being matched
in the ad-file.

All the other changes are ppc-specific.

Thank you and best regards,
Volker


[1] http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-December/thread.html#20400