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

[Vladimir Kozlov]

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