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

[Jatin Bhateja]

On Tue, 3 Jun 2025 13:30:05 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> src/hotspot/share/opto/intrinsicnode.cpp line 241:
>> 
>>> 239:   jlong lo = bt == T_INT ? min_jint : min_jlong;
>>> 240: 
>>> 241:   if(mask_type->is_con() && mask_type->get_con_as_long(bt) != -1L) {
>> 
>> Now you removed the condition `mask_type->get_con_as_long(bt) != -1L`. Do you know why it was there in the first place?
>> 
>> It seems to me that if `mask_type->get_con_as_long(bt) == -1L`, then we can just return the type of `src`, right?
>
> This is a bug-fix for `CompressBitsNode::Value`, but this change also has an effect on `ExpandBitsNode::Value`, and that makes me a little nervous. For example: do we have enough test coverage for `expand`? It seems we did not have enough tests for `compress`, so probably also not for `expand`...

> Now you removed the condition `mask_type->get_con_as_long(bt) != -1L`. Do you know why it was there in the first place?
> 
> It seems to me that if `mask_type->get_con_as_long(bt) == -1L`, then we can just return the type of `src`, right?

Correct, also for non-constant masks we can even find the maximum set bit count of entier value range and then estimate the bounds of the result.


#include <stdint.h>
#include <stdio.h>
#include <assert.h>
#include <stdlib.h>

uint64_t popcnt(int64_t val) {
  uint64_t res = 0;
  asm volatile("popcntq %1, %0 " : "=r"(res) : "r"(val) : "cc");
  return res;
}

typedef struct _result {
  int64_t value;
  int64_t mask;
  int64_t count;
} result;

result compute_max_mask(int64_t hi, int64_t lo) {
  assert(hi >= lo);
  result res;
  int64_t max_true_bits = 0;
  int64_t max_true_bits_val = 0;
  for (int64_t iter = lo; iter < hi; iter++) {
    int setbitscnt = popcnt(iter);
    if (max_true_bits < setbitscnt) {
      max_true_bits = setbitscnt;
      max_true_bits_val = iter;
    }
  }
  res.value = max_true_bits_val;
  res.mask = (1L << max_true_bits) - 1L;
  res.count = max_true_bits;
  return res;
}

int main(int argc, char* argv[]) {
  if (argc != 3) {
     return printf("Invalid arguments, <app> [lo] [hi]\n");
  }
  int64_t lo = atol(argv[1]);
  int64_t hi = atol(argv[2]);
  result mask = compute_max_mask(hi, lo);
  return printf("[lo] = %ld [hi] = %ld [set bits count] %ld [mask] = %ld [max bits count value] %ld\n",
                lo, hi, mask.count, mask.mask, mask.value);
}


For this bug fix patch I don't want to include this extension.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23947#discussion_r2204754331