is_power_of_2() has Undefined Behaviour and is inconsistent

is_power_of_2() has always behaved oddly, and it still behaves oddly but in a different way since 8183574: Unify the is_power_of_2 functions. And this change breaks TCK on AArch64 systems.

Previously, is_power_of_2(long) returned true for LONG_MIN, the most -ve signed long, but returned false for INT_MIN, the most -ve signed int.

Also, is_power_of_2(long) had Undefined Behaviour (UB) for LONG_MIN: it overflowed from the most -ve long to the most +ve long.

Now, is_power_of_2 has Undefined Behaviour for both int and long arguments when applied to their respective most negative values. In testing it returns true for both, but as this is UB it is not guaranteed.

previous definition:

inline bool is_power_of_2(intptr_t x) {
  return ((x != NoBits) && (mask_bits(x, x - 1) == NoBits));
}

since JDK-8240615:

template <typename T>
bool is_power_of_2(T x) {
  return (x != T(0)) && ((x & (x - 1)) == T(0));
}

I do not believe that this behavioural change was deliberate.

I suggest that we should change the definition of is_power_of_2() for all signed types such that it returns false for negative arguments. This makes the behaviour consistent and also avoids undefined behaviour.

This is important because is_power_of_2 is sometimes used as a guard in ADfile patterns:

instruct cmpI_branch_bit(cmpOpEqNe cmp, iRegIorL2I op1, immI op2, immI0 op3, label labl) %{
  match(If cmp (CmpI (AndI op1 op2) op3));
  predicate(is_power_of_2(n->in(2)->in(1)->in(2)->get_int()));
  effect(USE labl);

  ins_cost(BRANCH_COST);
  format %{ "tb$cmp $op1, $op2, $labl" %}
  ins_encode %{
    Label* L = $labl$$label;
    Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
    int bit = exact_log2($op2$$constant);
   ...

This has a runtime error since JDK-8240615 because the initial is_power_of_2 test succeeds, but the later exact_log2 fails. exact_log2() takes an intptr_t, not an int. On 64-bit systems, intptr_t is a 64-bit signed type.