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

[Dean Long]

Michael, isn't the correct size for this table 256?  I missed how VecZ 
relates to the table size.

dl

On 3/31/2016 9:58 AM, Berg, Michael C wrote:
> Up until now we have gotten along with the size constraint only.
> Let us have both the size and the table though for completeness.
> I think we can leave the name though.
>
> -Michael
>
> -----Original Message-----
> From: Rahul Raghavan [mailto:rahul.v.raghavan at oracle.com]
> Sent: Thursday, March 31, 2016 9:18 AM
> To: Dean Long <dean.long at oracle.com>; hotspot-compiler-dev at openjdk.java.net; Berg, Michael C <michael.c.berg at intel.com>
> Subject: RE: RFR(S): 8149488: Incorrect declaration of bitsInByte in regmask.cpp
>
> 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
>>>>>>