changeset 59359:e50512f91026

8240615: is_power_of_2() has Undefined Behaviour and is inconsistent Reviewed-by: jrose, redestad
author aph
date Tue, 10 Mar 2020 10:49:01 -0400
parents 12eb1e2087d2
children 65f30e209890 d0ff3ee1bf40
files src/hotspot/cpu/aarch64/aarch64.ad src/hotspot/cpu/x86/x86_64.ad src/hotspot/share/utilities/powerOfTwo.hpp test/hotspot/gtest/utilities/test_powerOfTwo.cpp
diffstat 4 files changed, 16 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/cpu/aarch64/aarch64.ad	Wed Mar 04 18:08:09 2020 +0100
+++ b/src/hotspot/cpu/aarch64/aarch64.ad	Tue Mar 10 10:49:01 2020 -0400
@@ -14936,7 +14936,7 @@
 
 instruct cmpL_branch_bit(cmpOpEqNe cmp, iRegL op1, immL op2, immL0 op3, label labl) %{
   match(If cmp (CmpL (AndL op1 op2) op3));
-  predicate(is_power_of_2(n->in(2)->in(1)->in(2)->get_long()));
+  predicate(is_power_of_2((julong)n->in(2)->in(1)->in(2)->get_long()));
   effect(USE labl);
 
   ins_cost(BRANCH_COST);
@@ -14944,7 +14944,7 @@
   ins_encode %{
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    int bit = exact_log2($op2$$constant);
+    int bit = exact_log2_long($op2$$constant);
     __ tbr(cond, $op1$$Register, bit, *L);
   %}
   ins_pipe(pipe_cmp_branch);
@@ -14953,7 +14953,7 @@
 
 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()));
+  predicate(is_power_of_2((juint)n->in(2)->in(1)->in(2)->get_int()));
   effect(USE labl);
 
   ins_cost(BRANCH_COST);
@@ -14961,7 +14961,7 @@
   ins_encode %{
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    int bit = exact_log2($op2$$constant);
+    int bit = exact_log2((juint)$op2$$constant);
     __ tbr(cond, $op1$$Register, bit, *L);
   %}
   ins_pipe(pipe_cmp_branch);
@@ -15001,7 +15001,7 @@
 
 instruct far_cmpL_branch_bit(cmpOpEqNe cmp, iRegL op1, immL op2, immL0 op3, label labl) %{
   match(If cmp (CmpL (AndL op1 op2) op3));
-  predicate(is_power_of_2(n->in(2)->in(1)->in(2)->get_long()));
+  predicate(is_power_of_2((julong)n->in(2)->in(1)->in(2)->get_long()));
   effect(USE labl);
 
   ins_cost(BRANCH_COST);
@@ -15009,7 +15009,7 @@
   ins_encode %{
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    int bit = exact_log2($op2$$constant);
+    int bit = exact_log2_long($op2$$constant);
     __ tbr(cond, $op1$$Register, bit, *L, /*far*/true);
   %}
   ins_pipe(pipe_cmp_branch);
@@ -15017,7 +15017,7 @@
 
 instruct far_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()));
+  predicate(is_power_of_2((juint)n->in(2)->in(1)->in(2)->get_int()));
   effect(USE labl);
 
   ins_cost(BRANCH_COST);
@@ -15025,7 +15025,7 @@
   ins_encode %{
     Label* L = $labl$$label;
     Assembler::Condition cond = (Assembler::Condition)$cmp$$cmpcode;
-    int bit = exact_log2($op2$$constant);
+    int bit = exact_log2((juint)$op2$$constant);
     __ tbr(cond, $op1$$Register, bit, *L, /*far*/true);
   %}
   ins_pipe(pipe_cmp_branch);
--- a/src/hotspot/cpu/x86/x86_64.ad	Wed Mar 04 18:08:09 2020 +0100
+++ b/src/hotspot/cpu/x86/x86_64.ad	Tue Mar 10 10:49:01 2020 -0400
@@ -3120,7 +3120,7 @@
 
 operand immL_Pow2()
 %{
-  predicate(is_power_of_2(n->get_long()));
+  predicate(is_power_of_2((julong)n->get_long()));
   match(ConL);
 
   op_cost(15);
@@ -3130,7 +3130,7 @@
 
 operand immL_NotPow2()
 %{
-  predicate(is_power_of_2(~n->get_long()));
+  predicate(is_power_of_2((julong)~n->get_long()));
   match(ConL);
 
   op_cost(15);
@@ -10029,7 +10029,7 @@
   ins_cost(125);
   format %{ "btsq    $dst, log2($con)\t# long" %}
   ins_encode %{
-    __ btsq($dst$$Address, log2_long($con$$constant));
+    __ btsq($dst$$Address, log2_long((julong)$con$$constant));
   %}
   ins_pipe(ialu_mem_imm);
 %}
--- a/src/hotspot/share/utilities/powerOfTwo.hpp	Wed Mar 04 18:08:09 2020 +0100
+++ b/src/hotspot/share/utilities/powerOfTwo.hpp	Tue Mar 10 10:49:01 2020 -0400
@@ -36,7 +36,7 @@
 
 template <typename T>
 bool is_power_of_2(T x) {
-  return (x != T(0)) && ((x & (x - 1)) == T(0));
+  return (x > T(0)) && ((x & (x - 1)) == T(0));
 }
 
 // Log2 of a power of 2
--- a/test/hotspot/gtest/utilities/test_powerOfTwo.cpp	Wed Mar 04 18:08:09 2020 +0100
+++ b/test/hotspot/gtest/utilities/test_powerOfTwo.cpp	Tue Mar 10 10:49:01 2020 -0400
@@ -36,6 +36,10 @@
   EXPECT_FALSE(is_power_of_2(T(0)));
   EXPECT_FALSE(is_power_of_2(~T(0)));
 
+  if (IsSigned<T>::value) {
+    EXPECT_FALSE(is_power_of_2(std::numeric_limits<T>::min()));
+  }
+
   // Test true
   for (T i = max_pow2<T>(); i > 0; i = (i >> 1)) {
     EXPECT_TRUE(is_power_of_2(i)) << "value = " << T(i);