From ebeddef9149f4a57749d06ddd74f39a8874d68a4 Mon Sep 17 00:00:00 2001 From: Nils Eliasson Date: Thu, 20 May 2021 11:02:05 +0200 Subject: [PATCH 1/5] fix xor value --- src/hotspot/share/opto/addnode.cpp | 29 ++++ .../jtreg/compiler/types/TestMeetXor.java | 136 ++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 test/hotspot/jtreg/compiler/types/TestMeetXor.java diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index 091a09b9b56d1..d3a6a458fdbe7 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -920,9 +920,24 @@ const Type* XorINode::Value(PhaseGVN* phase) const { if (in1->eqv_uncast(in2)) { return add_id(); } + // result of xor can only have bits sets where any of the + // inputs have bits set. lo can always become 0. + const TypeInt* t1i = t1->is_int(); + const TypeInt* t2i = t2->is_int(); + if ((t1i->_lo >= 0) && + (t1i->_hi > 0) && + (t1i->_hi <= max_power_of_2()) && + (t2i->_lo >= 0) && + (t2i->_hi > 0) && + (t2i->_hi <= max_power_of_2())) { + const TypeInt* t1x = TypeInt::make(0, next_power_of_2(t1i->_hi) - 1, t1i->_widen); + const TypeInt* t2x = TypeInt::make(0, next_power_of_2(t2i->_hi) - 1, t2i->_widen); + return t1x->meet(t2x); + } return AddNode::Value(phase); } + //------------------------------add_ring--------------------------------------- // Supplied function returns the sum of the inputs IN THE CURRENT RING. For // the logical operations the ring's ADD is really a logical OR function. @@ -970,6 +985,20 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { if (in1->eqv_uncast(in2)) { return add_id(); } + // result of xor can only have bits sets where any of the + // inputs have bits set. lo can always become 0. + const TypeLong* t1l = t1->is_long(); + const TypeLong* t2l = t2->is_long(); + if ((t1l->_lo >= 0) && + (t1l->_hi > 0) && + (t1l->_hi <= max_power_of_2()) && + (t2l->_lo >= 0) && + (t2l->_hi > 0) && + (t2l->_hi <= max_power_of_2())) { + const TypeLong* t1x = TypeLong::make(0, next_power_of_2(t1l->_hi) - 1, t1l->_widen); + const TypeLong* t2x = TypeLong::make(0, next_power_of_2(t2l->_hi) - 1, t2l->_widen); + return t1x->meet(t2x); + } return AddNode::Value(phase); } diff --git a/test/hotspot/jtreg/compiler/types/TestMeetXor.java b/test/hotspot/jtreg/compiler/types/TestMeetXor.java new file mode 100644 index 0000000000000..400537a673593 --- /dev/null +++ b/test/hotspot/jtreg/compiler/types/TestMeetXor.java @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8267332 + * @summary Test meet on xor + * @library /test/lib / + * @run main/othervm compiler.types.TestMeetXor -XX::CompileCommand=dontinline,*::test* + */ + +package compiler.types; + +import java.util.Random; +import jdk.test.lib.Asserts; + +public class TestMeetXor { + public static void main(String[] args) throws Exception { + for (int i = 0; i < 10000; i++) { + testCase1E(); + testCase2E(); + testCase3E(); + + testCaseS(); + } + } + + static int[] count = new int[256]; + static Random r = new Random(5); + + public static void testCase1E() { + boolean aiobe = false; + try { + testBound1E(r.nextInt()); + } catch (ArrayIndexOutOfBoundsException e) { + aiobe = true; + } + Asserts.assertTrue(aiobe); + } + + public static void testBound1E(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ -1; + count[index]++; + } + + public static void testCase2E() { + boolean aiobe = false; + try { + testBound2E(r.nextInt()); + } catch (ArrayIndexOutOfBoundsException e) { + aiobe = true; + } + Asserts.assertTrue(aiobe); + } + + public static void testBound2E(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ 0x100; + count[index]++; + } + + public static void testCase3E() { + boolean aiobe = false; + try { + testBound3E(r.nextInt()); + } catch (ArrayIndexOutOfBoundsException e) { + aiobe = true; + } + Asserts.assertTrue(aiobe); + } + + public static void testBound3E(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ Integer.MIN_VALUE; + count[index]++; + } + + public static void testCase4E() { + boolean aiobe = false; + try { + testBound4E(r.nextInt(), 0xf0f0ff); + } catch (ArrayIndexOutOfBoundsException e) { + aiobe = true; + } + Asserts.assertTrue(aiobe); + } + + public static void testBound4E(int i1, int i2) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ i2; + count[index]++; + } + + public static void testCaseS() { + testBound1S(r.nextInt()); + testBound2S(r.nextInt()); + testBound3S(r.nextInt(), r.nextInt()); + } + + public static void testBound1S(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ 0x80; + count[index]++; + } + + public static void testBound2S(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ 0xff; + count[index]++; + } + + public static void testBound3S(int i1, int i2) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ (i2 >>> 24); + count[index]++; + } + + public static void testBound4S(int i1) throws ArrayIndexOutOfBoundsException { + int index = (i1 >>> 24) ^ 0; + count[index]++; + } +} From 5f975338b387fc0068854804c7a2ea2540c90c70 Mon Sep 17 00:00:00 2001 From: Nils Eliasson Date: Thu, 20 May 2021 23:46:35 +0200 Subject: [PATCH 2/5] Fix bounds check --- src/hotspot/share/opto/addnode.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index d3a6a458fdbe7..d03354c472c01 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -926,10 +926,10 @@ const Type* XorINode::Value(PhaseGVN* phase) const { const TypeInt* t2i = t2->is_int(); if ((t1i->_lo >= 0) && (t1i->_hi > 0) && - (t1i->_hi <= max_power_of_2()) && + (t1i->_hi < max_power_of_2()) && (t2i->_lo >= 0) && (t2i->_hi > 0) && - (t2i->_hi <= max_power_of_2())) { + (t2i->_hi < max_power_of_2())) { const TypeInt* t1x = TypeInt::make(0, next_power_of_2(t1i->_hi) - 1, t1i->_widen); const TypeInt* t2x = TypeInt::make(0, next_power_of_2(t2i->_hi) - 1, t2i->_widen); return t1x->meet(t2x); @@ -991,10 +991,10 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { const TypeLong* t2l = t2->is_long(); if ((t1l->_lo >= 0) && (t1l->_hi > 0) && - (t1l->_hi <= max_power_of_2()) && + (t1l->_hi < max_power_of_2()) && (t2l->_lo >= 0) && (t2l->_hi > 0) && - (t2l->_hi <= max_power_of_2())) { + (t2l->_hi < max_power_of_2())) { const TypeLong* t1x = TypeLong::make(0, next_power_of_2(t1l->_hi) - 1, t1l->_widen); const TypeLong* t2x = TypeLong::make(0, next_power_of_2(t2l->_hi) - 1, t2l->_widen); return t1x->meet(t2x); From c7fecda8fcb50109bdde14398e7ed18c7cf534b4 Mon Sep 17 00:00:00 2001 From: Nils Eliasson Date: Thu, 20 May 2021 23:54:26 +0200 Subject: [PATCH 3/5] fixed missing test case --- test/hotspot/jtreg/compiler/types/TestMeetXor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/test/hotspot/jtreg/compiler/types/TestMeetXor.java b/test/hotspot/jtreg/compiler/types/TestMeetXor.java index 400537a673593..6ef6b0eb0dfca 100644 --- a/test/hotspot/jtreg/compiler/types/TestMeetXor.java +++ b/test/hotspot/jtreg/compiler/types/TestMeetXor.java @@ -112,6 +112,7 @@ public static void testCaseS() { testBound1S(r.nextInt()); testBound2S(r.nextInt()); testBound3S(r.nextInt(), r.nextInt()); + testBound4S(r.nextInt()); } public static void testBound1S(int i1) throws ArrayIndexOutOfBoundsException { From 4d856ec103bac388bdf2491327740ce49d678e42 Mon Sep 17 00:00:00 2001 From: Nils Eliasson Date: Fri, 21 May 2021 11:28:13 +0200 Subject: [PATCH 4/5] Updated test, fixed bound --- src/hotspot/share/opto/addnode.cpp | 10 ++++++---- test/hotspot/jtreg/compiler/types/TestMeetXor.java | 8 +++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index d03354c472c01..a6ec5ed8b04f8 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -930,8 +930,9 @@ const Type* XorINode::Value(PhaseGVN* phase) const { (t2i->_lo >= 0) && (t2i->_hi > 0) && (t2i->_hi < max_power_of_2())) { - const TypeInt* t1x = TypeInt::make(0, next_power_of_2(t1i->_hi) - 1, t1i->_widen); - const TypeInt* t2x = TypeInt::make(0, next_power_of_2(t2i->_hi) - 1, t2i->_widen); + // hi - set all bits below the highest bit. Using round_down to avoid overflow. + const TypeInt* t1x = TypeInt::make(0, round_down_power_of_2(t1i->_hi) + (round_down_power_of_2(t1i->_hi) - 1), t1i->_widen); + const TypeInt* t2x = TypeInt::make(0, round_down_power_of_2(t2i->_hi) + (round_down_power_of_2(t2i->_hi) - 1), t2i->_widen); return t1x->meet(t2x); } return AddNode::Value(phase); @@ -995,8 +996,9 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { (t2l->_lo >= 0) && (t2l->_hi > 0) && (t2l->_hi < max_power_of_2())) { - const TypeLong* t1x = TypeLong::make(0, next_power_of_2(t1l->_hi) - 1, t1l->_widen); - const TypeLong* t2x = TypeLong::make(0, next_power_of_2(t2l->_hi) - 1, t2l->_widen); + // hi - set all bits below the highest bit. Using round_down to avoid overflow. + const TypeLong* t1x = TypeLong::make(0, round_down_power_of_2(t1l->_hi) + (round_down_power_of_2(t1l->_hi) - 1), t1l->_widen); + const TypeLong* t2x = TypeLong::make(0, round_down_power_of_2(t2l->_hi) + (round_down_power_of_2(t2l->_hi) - 1), t2l->_widen); return t1x->meet(t2x); } return AddNode::Value(phase); diff --git a/test/hotspot/jtreg/compiler/types/TestMeetXor.java b/test/hotspot/jtreg/compiler/types/TestMeetXor.java index 6ef6b0eb0dfca..c25d22c73e132 100644 --- a/test/hotspot/jtreg/compiler/types/TestMeetXor.java +++ b/test/hotspot/jtreg/compiler/types/TestMeetXor.java @@ -23,10 +23,11 @@ /* * @test + * @key randomness * @bug 8267332 * @summary Test meet on xor * @library /test/lib / - * @run main/othervm compiler.types.TestMeetXor -XX::CompileCommand=dontinline,*::test* + * @run main/othervm compiler.types.TestMeetXor -Xbatch -XX::CompileCommand=dontinline,*::test* */ package compiler.types; @@ -36,17 +37,18 @@ public class TestMeetXor { public static void main(String[] args) throws Exception { - for (int i = 0; i < 10000; i++) { + for (int i = 0; i < 50_000; i++) { testCase1E(); testCase2E(); testCase3E(); + testCase4E(); testCaseS(); } } static int[] count = new int[256]; - static Random r = new Random(5); + static Random r = jdk.test.lib.Utils.getRandomInstance(); public static void testCase1E() { boolean aiobe = false; From 9be3b003645b8013e02649d6c7ff8b554d88366f Mon Sep 17 00:00:00 2001 From: Nils Eliasson Date: Fri, 21 May 2021 11:42:21 +0200 Subject: [PATCH 5/5] Removed unnecessary check --- src/hotspot/share/opto/addnode.cpp | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/hotspot/share/opto/addnode.cpp b/src/hotspot/share/opto/addnode.cpp index a6ec5ed8b04f8..2e9da21ca6b31 100644 --- a/src/hotspot/share/opto/addnode.cpp +++ b/src/hotspot/share/opto/addnode.cpp @@ -924,12 +924,10 @@ const Type* XorINode::Value(PhaseGVN* phase) const { // inputs have bits set. lo can always become 0. const TypeInt* t1i = t1->is_int(); const TypeInt* t2i = t2->is_int(); - if ((t1i->_lo >= 0) && - (t1i->_hi > 0) && - (t1i->_hi < max_power_of_2()) && - (t2i->_lo >= 0) && - (t2i->_hi > 0) && - (t2i->_hi < max_power_of_2())) { + if ((t1i->_lo >= 0) && + (t1i->_hi > 0) && + (t2i->_lo >= 0) && + (t2i->_hi > 0)) { // hi - set all bits below the highest bit. Using round_down to avoid overflow. const TypeInt* t1x = TypeInt::make(0, round_down_power_of_2(t1i->_hi) + (round_down_power_of_2(t1i->_hi) - 1), t1i->_widen); const TypeInt* t2x = TypeInt::make(0, round_down_power_of_2(t2i->_hi) + (round_down_power_of_2(t2i->_hi) - 1), t2i->_widen); @@ -990,12 +988,10 @@ const Type* XorLNode::Value(PhaseGVN* phase) const { // inputs have bits set. lo can always become 0. const TypeLong* t1l = t1->is_long(); const TypeLong* t2l = t2->is_long(); - if ((t1l->_lo >= 0) && - (t1l->_hi > 0) && - (t1l->_hi < max_power_of_2()) && - (t2l->_lo >= 0) && - (t2l->_hi > 0) && - (t2l->_hi < max_power_of_2())) { + if ((t1l->_lo >= 0) && + (t1l->_hi > 0) && + (t2l->_lo >= 0) && + (t2l->_hi > 0)) { // hi - set all bits below the highest bit. Using round_down to avoid overflow. const TypeLong* t1x = TypeLong::make(0, round_down_power_of_2(t1l->_hi) + (round_down_power_of_2(t1l->_hi) - 1), t1l->_widen); const TypeLong* t2x = TypeLong::make(0, round_down_power_of_2(t2l->_hi) + (round_down_power_of_2(t2l->_hi) - 1), t2l->_widen);