RFR: 8143219: AArch64 broken by 8141132: JEP 254: Compact Strings

[Andrew Haley]

On 11/19/2015 03:33 PM, Edward Nevill wrote:
> On Thu, 2015-11-19 at 11:56 +0000, Andrew Haley wrote:
>> http://cr.openjdk.java.net/~aph/8143219/
>>
>> 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.

Yes.

> 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.

Andrew.