Details

Bug

Resolution: Fixed

P3

15

b14
Description
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));
}
sinceJDK8240615:
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 sinceJDK8240615 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 64bit systems, intptr_t is a 64bit signed type.
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
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
Attachments
Issue Links
 duplicates

JDK8239861 [aarch64] assert(is_power_of_2(x)) failed: x must be a power of 2: 0xffffffff80000000
 Closed