[9] RFR(S): 8144693: Intrinsify StringCoding.hasNegatives() on SPARC

[John Rose]

This is the sort of low-level vector loop that we should be able to do routinely in Java/unsafe code.
In this case bootstrapping makes it hard (anything in String has to be easy to start up).
— John

On Mar 15, 2016, at 8:53 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Somehow I missed this mail.
> 
> Changes looks good now.
> 
> Thanks,
> Vladimir
> 
> On 3/7/16 9:30 AM, Guy Delamarter wrote:
>> Hi,
>> 
>> On 03/02/2016 08:37 PM, Vladimir Kozlov wrote:
>>> Why you are avoiding the same register for src and dst in
>>> instructions? For example:
>>> 
>>> +  add(t3, 0x80, t2);
>>> 
>>> I think you can use less registers if you reuse registers. The less
>>> registers you use - less registers will be spilled on stack in the
>>> code which use your code.
>> 
>> Spilling doesn't appear to be a problem in actual hasNegatives use. In
>> the cases I've looked at, on entry the second parameter, size, is copied
>> from one register (e.g. %i3) to %g3 and both are preserved all the way
>> through the hasNegatives code, without prior spill.  The former register
>> is then the one used after exit.  If register pressure and resulting
>> spilling becomes an issue, hopefully there is some way to eliminate the
>> duplication.
>> 
>>> 
>>> 
>>> 'size' register is defined as 32-bit values in .ad file. It may have
>>> garbage in apper bits - I would suggest to sign extend it before using:
>>> 
>> Good to know.
>> 
>>> signx(size);
>>> 
>> 
>> 
>>> If size is small (t5 != 0 for unaligned header) may be we should just
>>> jump to tail code to do right shift (to mov(0, result); instruction).
>>> But I am not insisting on this.
>> 
>> If I understand correctly that replaces two instructions with three, one
>> of which is a branch, without providing additional function.
>> 
>> I made the other changes, and after I noticed the minor problem, fixed
>> it so unaligned array segments of size 0 won't perform any reads.
>> 
>> New Webrevs:
>> 
>> http://cr.openjdk.java.net/~thartmann/8144693/hotspot/webrev.01/
>> http://cr.openjdk.java.net/~thartmann/8144693/jdk/webrev.01/
>> 
>> 
>> Thanks,
>> Guy Delamarter
>> 
>>> 
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 3/2/16 3:33 PM, Guy Delamarter wrote:
>>>> Hi,
>>>> 
>>>> Please review the following patch adding a SPARC intrinsic for
>>>> StringCoding.hasNegatives to hotspot and supporting changes.
>>>> 
>>>> Tobias Hartmann (tobias.hartmann at oracle.com) has offered to
>>>> sponsor these changes.
>>>> 
>>>> 
>>>> Summary:
>>>> 
>>>> hasNegatives is a utility method for encoding/decoding under the new
>>>> Compact Strings framework, returning whether the given byte array
>>>> (segment) contains any negative entries.  hasNegatives is an intrinsic
>>>> candidate, emitted by hotspot as a node in the latest Java 9 builds,
>>>> but the SPARC node match and (inline assembly) implementation have
>>>> been missing up to now.  This implementation checks aligned 8-byte
>>>> windows onto the array segment for negative bytes, carefully ignoring
>>>> bytes in the window that happen to be outside the segment.
>>>> 
>>>> It was discovered that the private access of the hasNegatives method
>>>> prevented direct testing of the intrinsic from outside StringCoding,
>>>> so that was changed to public.  Subsequent to this change a compiler
>>>> test class (TestHasNegatives) was added providing useful validation
>>>> coverage.
>>>> 
>>>> 
>>>> Webrevs:
>>>> http://cr.openjdk.java.net/~thartmann/8144693/hotspot/webrev.00/
>>>> http://cr.openjdk.java.net/~thartmann/8144693/jdk/webrev.00/
>>>> 
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8144693
>>>> 
>>>> 
>>>> Thanks,
>>>> Guy Delamarter
>>>> 
>>