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

[Guy Delamarter]

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