RFR: 8143219: AArch64 broken by 8141132: JEP 254: Compact Strings
aph at redhat.com
Thu Nov 19 15:57:08 UTC 2015
On 11/19/2015 03:33 PM, Edward Nevill wrote:
> On Thu, 2015-11-19 at 11:56 +0000, Andrew Haley wrote:
>> Two things happened, one simple and one not-so-simple.
>> Firstly, the string comparators now take a byte count (because it's a
>> byte array now) rather than a char count.
>> Secondly, string_indexOf needs to have a guard to make sure that
>> substr.count <= string.count. This is guaranteed by
>> LibraryCallKit::inline_string_indexOf but not by
>> LibraryCallKit::inline_string_indexOfI. This is a subtle change, and
>> was quite tricky to figure out. I have fixed it here in the AArch64
>> back end.
>> It would be nice to have the preconditions for the intrinsics
>> commented somewhere, so that a porter could know what they should do
>> and what they should expect.
> Two things
> 1) Is it really the case that string_compare takes the inputs as a
> byte count but returns the result as a char count which AFAICS is
> what this does.
> It would be nice to know the post-conditions as well.
> 2) For efficiency the asr cnt, #1 instructions could be folded into
> the implementations of string_compare and string_equals.
They could, and I looked at doing so, but for the sake of robustness
and not changing the interface to the intrinsic generators I didn't.
If (God forbid) there was ever an odd byte length in that array the
intrinsic would scan all of memory, which is a very different kind of
badness from the Java code. So, I thought I'd mask out the lower bit,
but that's the same instruction count. Given that the performance
difference is so slight I went with robustness.
Incidentally, I noticed that we have a char_arrays_equals which seems
to do the same job as string_equals. I think x86 uses the same
primitive for both, and we should too.
More information about the hotspot-dev