RFR: 8350896: Integer/Long.compress gets wrong type from CompressBitsNode::Value [v8]

[Jatin Bhateja]

On Tue, 3 Jun 2025 14:24:31 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> We can further constrain the value range bounds of bit compression and expansion once PR #17508 gets integrated. For now, I have developed the following draft demonstrates bound constraining with KnownBitLattice.
>> 
>> 
>> //
>> // Prototype of bit compress/expand value range computation
>> // using KnownBits infrastructure.
>> //
>> 
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <stdint.h>
>> #include <assert.h>
>> 
>> template<class U,class S>
>> class KnownBitsLattice {
>>   private:
>>     U zeros;
>>     U ones;
>> 
>>   public:
>>     KnownBitsLattice(U lb, U ub);
>> 
>>     U getKnownZeros() {
>>       return zeros;
>>     }
>> 
>>     U getKnownOnes() {
>>       return ones;
>>     }
>> 
>>     long getKnownZerosCount() {
>>       uint64_t count = 0;
>>       asm volatile ("popcntq %1, %0 \n\t" : "=r"(count) : "r"(zeros) : "cc");
>>       return count;
>>     }
>> 
>>     long getKnownOnesCount() {
>>       uint64_t count = 0;
>>       asm volatile ("popcntq %1, %0 \n\t" : "=r"(count) : "r"(ones) : "cc");
>>       return count;
>>     }
>> 
>>     bool check_voilation() {
>>       // A given bit cannot be both zero or one.
>>       return (zeros & ones) != 0;
>>     }
>> 
>>     bool is_MSB_KnownOneBitsSet() {
>>       return (ones >> 63) == 1;
>>     }
>> 
>>     bool is_MSB_KnownZeroBitsSet() {
>>       return (zeros >> 63) == 1;
>>     }
>> };
>> 
>> template<class U, class S>
>> KnownBitsLattice<U,S>::KnownBitsLattice(U lb, U ub) {
>>   // To find KnownBitsLattice from a given value range
>>   // we first find the common prefix b/w upper and lower
>>   // bound, we then concertize known zeros and ones bit
>>   // based on common prefix.
>>   // e.g.
>>   //    lb = 00110001
>>   //    ub = 00111111
>>   //    common prefix = 0011XXXX
>>   //    knownbits.zeros = 11000000
>>   //    knownbits.ones = 00110000
>>   //
>>   // conversely, for a give knownbits value we can find
>>   // lower and upper value ranges.
>>   // e.g.
>>   //      knownbits.zeros = 0x00010001
>>   //      knownbits.ones  = 0x10001100
>>   //  range.lo = knownbits.ones, this is because knownbits.ones are
>>   //                             guaranteed to be one.
>>   //  range.hi = ~knownbits.zeros, this is an optimistic upper bound
>>   //                               which assumes all unset knownbits.zero
>>   //                               are ones.
>>   // Thus in above example,
>>   // range.lo = 0x8C
>>   // range.hi = 0xEE
>> 
>>   U lzcnt = 0;
>>   U common_prefix = lb ^ ub;
>>   asm volatile ("lzcntq %1, %0 \n\t" : "=r"(lzcnt) : "r"...
>
> @jatin-bhateja I think we are making progress, it seems to me now that the VM code is correct, at least as far as I can tell with visual inspection.
> 
> We are still missing some additional tests, as I have asked for a few times already:
> https://github.com/openjdk/jdk/pull/23947#issuecomment-2853896251
> 
> We should do something like this:
> 
> public static test(int mask, int src) {
>   mask = Math.max(CON1, Math.min(CON2, mask));
>   src = Math.max(CON2, Math.min(CON4, src));
>   result = Integer.compress(src, mask);
>   int sum = 0;
>   if (sum > LIMIT_1) { sum += 1; }
>   if (sum > LIMIT_2) { sum += 2; }
>   if (sum > LIMIT_3) { sum += 4; }
>   if (sum > LIMIT_4) { sum += 8; }
>   if (sum > LIMIT_5) { sum += 16; }
>   if (sum > LIMIT_6) { sum += 32; }
>   if (sum > LIMIT_7) { sum += 64; }
>   if (sum > LIMIT_8) { sum += 128; }
>   return new int[] {sum, result};
> }
> 
> 
> You could do the same pattern for `expand` too.
> Then you pick random values using `Generators.java` for all the `CON` and `LIMIT`.
> If we somehow produce a bad range, then the limit checks could constant fold wrongly, and then the `sum` would reflect this wrong result. Optimal would be to duplicate this pattern, and have one method that compiles, and one that runs in interpreter. That way, you can repeatedly call the methods with various `src` and `mask` values, and compare the output.

Hi @eme64 , 

Updated the tests as per suggestion; however, for this bug fix patch, we are not doing aggressive value range optimization.
I plan to extend value routines for compress/expand with the newly supported knownBits infrastructure in a subsequent RFE., Following is a prototype for the same.

https://github.com/jatin-bhateja/external_staging/blob/main/Code/java/knownBits_DFA/bit_compress_expand_KnownBits.java

Best Regards,
Jatin

-------------

PR Comment: https://git.openjdk.org/jdk/pull/23947#issuecomment-3062355806