RFR(S): 8149488: Incorrect declaration of bitsInByte in regmask.cpp

[Rahul Raghavan]

Hi Michael,

With respect to below thread, request help with some questions.
Understood that the bitsInByte[] lookup table tells the number of bits set for the looked-up number and is useful in VectorSet Size.
Also comment got was for requirement to extend bitsInByte table to 512 size, for consistent mapping for VecZ register also, on targets that support it.
But currently for bitsInByte used in VectorSet::Size(), RegMask::Size(), only the original 256 size is sufficient or useful here. 
Could not find usage of extended set of values in bitsInByte table! Is it for something to be used in future?

So for now it seems following original fix for 8149488 - only correcting the size at RegMask::Size(), seems to be okay? 
(without extending current bitsInByte array contents)
(Anyhow at present values above 0xFF is never indexed for bitsInByte in RegMask::Size())

         ----- src/share/vm/libadt/vectset.hpp
         +#define BITS_IN_BYTE_ARRAY_SIZE 256
         +

         ----- src/share/vm/opto/regmask.cpp
         -  extern uint8_t bitsInByte[512];
         +  extern uint8_t bitsInByte[BITS_IN_BYTE_ARRAY_SIZE];

         ----- src/share/vm/libadt/vectset.cpp
         -uint8_t bitsInByte[256] = {
         +uint8_t bitsInByte[BITS_IN_BYTE_ARRAY_SIZE] = {

I can send revised webrev for above if all okay. Please tell me if I am missing something.


OR else if extending bitsInByte array size and entries indeed is the correct fix, 
the array name also should be changed, right ? 
(to maybe 'bitCountArray[BIT_COUNT_ARRAY_SIZE]')

Thanks,
Rahul

> -----Original Message-----
> From: Rahul Raghavan > Sent: Thursday, March 31, 2016 6:49 PM
> 
> > -----Original Message-----
> > From: Dean Long > Sent: Thursday, March 31, 2016 1:05 AM
> >
> > When do we access elements 256 .. 511?  Wouldn't that mean we have 9-bit
> > bytes?
> 
> Got your point Dean, Thanks.
> I too got some questions here now; will check and reply soon.
> 
> -Rahul
> 
> >
> > dl
> >
> > On 3/30/2016 12:19 PM, Rahul Raghavan wrote:
> > > Hi,
> > >
> > > With respect to below email thread, request help to review revised webrev.01 for 8149488.
> > >
> > > <bug>: https://bugs.openjdk.java.net/browse/JDK-8149488
> > > <webrev.01>: http://cr.openjdk.java.net/~rraghavan/8149488/webrev.01/
> > >
> > > Now as required, fixed the issue by extending bitsInByte array size from 256 to 512.
> > > Defined and used BITS_IN_BYTE_ARRAY_SIZE for the array size as 512.
> > > Confirmed no issues with 'JPRT -testset hotspot' run.
> > >
> > > Thanks,
> > > Rahul
> > >
> > >> -----Original Message-----
> > >> From: Berg, Michael C [mailto:michael.c.berg at intel.com] > Sent: Thursday, February 11, 2016 11:32 PM > To: hotspot-compiler-
> > dev at openjdk.java.net
> > >>
> > >> Should we not extend:
> > >>
> > >> This does not match the actual definition in share/vm/libadt/vectset.cpp:
> > >>   uint8_t bitsInByte[256] = { // ...
> > >>
> > >> to 512
> > >>
> > >> -----Original Message-----
> > >> From: Berg, Michael C > Sent: Thursday, February 11, 2016 9:57 AM > To: 'Vladimir Ivanov'
> > >>
> > >> So how do we intend to map a VecZ register without 512 bits?
> > >>
> > >> -Michael
> > >>
> > >> -----Original Message-----
> > >> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Ivanov
> > >> Sent: Thursday, February 11, 2016 4:54 AM > To: Rahul Raghavan; hotspot-compiler-dev at openjdk.java.net
> > >>
> > >> Rahul,
> > >>
> > >> Can we define a constant instead and use it in both places?
> > >>
> > >> Best regards,
> > >> Vladimir Ivanov
> > >>
> > >> On 2/11/16 10:42 AM, Rahul Raghavan wrote:
> > >>> Hi,
> > >>>
> > >>> Please review the patch for JDK- 8149488.
> > >>>
> > >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8149488
> > >>> Webrev: http://cr.openjdk.java.net/~thartmann/8149488/webrev.00/
> > >>>
> > >>> Corrected the bitsInByte array size in declaration.
> > >>>
> > >>> Thanks,
> > >>> Rahul
> > >>>
> >