RFR(S): Bugfix: Clear High Word in Integer.bitCount() Intrinsic on SparcV9

[Reingruber, Richard]

Thanks, Vladimir and Chris, for your reviews and the bug id.

  > It would seem easier to just write two assembler instructions instead of using expand.

That's what I did first, and it was easier ;)

  > Is there a particular reason you are using expand?

With the expand there is a chance for a better schedule. When testing, I saw for example that the convI2L_reg_zex was scheduled into the delay slot of a predecessor block.

Is there a reason _not_ to use the expand?

And here's the webrev updated with the bug id:

   http://www.sapjvm.com/rr/webrevs/8005033_bitCount_intrinsic_sparc_02/

Cheers, Richard.

-----Original Message-----
From: hotspot-compiler-dev-bounces at openjdk.java.net [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Kozlov
Sent: Donnerstag, 13. Dezember 2012 20:48
To: hotspot-compiler-dev at openjdk.java.net
Subject: Re: RFR(S): Bugfix: Clear High Word in Integer.bitCount() Intrinsic on SparcV9

We should use macroassembler instructions and we have special 
instruction to clear upper half:

   ins_encode %{
     __ clruwu($src$$Register);
     __ popc($src$$Register, $dst$$Register);
   %}

Vladimir

On 12/13/12 10:46 AM, Christian Thalinger wrote:
>
> On Dec 13, 2012, at 9:09 AM, "Reingruber, Richard" <richard.reingruber at sap.com> wrote:
>
>> Hi,
>>
>> I would like to submit a bugfix for C2's Integer.bitCount() intrinsic on sparcv9.
>>
>> On sparcv9, the C2 intrinsic for Integer.bitCount() is a POPC instruction. POPC operates on the whole register, including the high word, but for Integer.bitCount(), this is not correct, because the value of the high word is undefined. There could be bits set as a result of an int overflow or from a shift operation for example. Of course the bits in the high word have to be ignored for Integer.bitCount().
>>
>> I have prepared a small webrev with the suggested fix and a regression test:
>>
>>    http://www.sapjvm.com/rr/webrevs/bitCount_intrinsic_sparc_01/
>>
>> Could you please review the patch and create a bug id? Thanks!
>
> Thanks for contributing this patch.  I added these instructions so I'll take care of it.  Here is the bug:
>
> 8005033: clear high word for integer pop count on SPARC
>
> It would seem easier to just write two assembler instructions instead of using expand.  Is there a particular reason you are using expand?
>
> -- Chris
>
>>
>> Cheers,
>> Richard.
>>
>> BTW: my name is Richard Reingruber, and I'm working as an engineer in the JIT team at SAP in Germany. This is my first posting, so a minimal introduction is probably appropriate :)
>>
>