From 98ae73c13c27d8d7b405f925729a297b1a27bb24 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Sat, 18 Oct 2025 20:57:03 +0200 Subject: [PATCH 1/4] test --- .../compiler/c2/gvn/ModINodeValueTests.java | 41 ++++++++++++++++++- .../compiler/c2/gvn/ModLNodeValueTests.java | 41 ++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/test/hotspot/jtreg/compiler/c2/gvn/ModINodeValueTests.java b/test/hotspot/jtreg/compiler/c2/gvn/ModINodeValueTests.java index 7870ce1091048..ae942f1704b1b 100644 --- a/test/hotspot/jtreg/compiler/c2/gvn/ModINodeValueTests.java +++ b/test/hotspot/jtreg/compiler/c2/gvn/ModINodeValueTests.java @@ -36,7 +36,7 @@ /* * @test - * @bug 8356813 + * @bug 8356813 8366815 * @summary Test that Value method of ModINode is working as expected. * @key randomness * @library /test/lib / @@ -54,6 +54,8 @@ public static void main(String[] args) { @Run(test = { "nonNegativeDividend", "nonNegativeDividendInRange", "negativeDividend", "negativeDividendInRange", + "positiveDivisor", "positiveDivisor2", + "negativeDivisor", "negativeDivisor2", "modByKnownBoundsUpper", "modByKnownBoundsUpperInRange", "modByKnownBoundsLower", "modByKnownBoundsLowerInRange", "modByKnownBoundsLimitedByDividendUpper", "modByKnownBoundsLimitedByDividendUpperInRange", @@ -79,6 +81,10 @@ public void assertResult(int x, int y) { Asserts.assertEQ(x != 0 && POS_INT % x <= 0, nonNegativeDividendInRange(x)); Asserts.assertEQ(x != 0 && NEG_INT % x > 0, negativeDividend(x)); Asserts.assertEQ(x != 0 && NEG_INT % x >= 0, negativeDividendInRange(x)); + Asserts.assertEQ(x % POS_INT >= POS_INT, positiveDivisor(x)); + Asserts.assertEQ(x % POS_INT <= -POS_INT, positiveDivisor2(x)); + Asserts.assertEQ(x % NEG_INT <= NEG_INT, negativeDivisor(x)); + Asserts.assertEQ(x % NEG_INT > -(NEG_INT + 1), negativeDivisor2(x)); Asserts.assertEQ(x % (((byte) y) + 129) > 255, modByKnownBoundsUpper(x, y)); Asserts.assertEQ(x % (((byte) y) + 129) >= 255, modByKnownBoundsUpperInRange(x, y)); Asserts.assertEQ(x % (((byte) y) + 129) < -255, modByKnownBoundsLower(x, y)); @@ -137,6 +143,39 @@ public boolean negativeDividendInRange(int x) { return x != 0 && NEG_INT % x >= 0; } + @Test + @IR(failOn = {IRNode.MOD_I, IRNode.CMP_I, IRNode.AND_I, IRNode.RSHIFT_I}) + // The result is always smaller than the positive divisor. + // Constant fold to false. + public boolean positiveDivisor(int x) { + return x % POS_INT >= POS_INT; + } + + @Test + @IR(failOn = {IRNode.MOD_I, IRNode.CMP_I, IRNode.AND_I, IRNode.RSHIFT_I}) + // The result is always bigger than the negated positive divisor. + // Constant fold to false. + public boolean positiveDivisor2(int x) { + return x % POS_INT <= -POS_INT; + } + + @Test + @IR(failOn = {IRNode.MOD_I, IRNode.CMP_I, IRNode.AND_I, IRNode.RSHIFT_I}) + // The result is always smaller than the negated negative divisor with exception if MIN_VALUE. + // Constant fold to false. + public boolean negativeDivisor(int x) { + // > with + 1 to avoid -MIN_VALUE == MIN_VALUE + return x % NEG_INT > -(NEG_INT + 1); + } + + @Test + @IR(failOn = {IRNode.MOD_I, IRNode.CMP_I, IRNode.AND_I, IRNode.RSHIFT_I}) + // The result is always bigger than the negative divisor. + // Constant fold to false. + public boolean negativeDivisor2(int x) { + return x % NEG_INT <= NEG_INT; + } + @Test @IR(failOn = {IRNode.MOD_I, IRNode.CMP_I}) // The magnitude of the result is less than the divisor. diff --git a/test/hotspot/jtreg/compiler/c2/gvn/ModLNodeValueTests.java b/test/hotspot/jtreg/compiler/c2/gvn/ModLNodeValueTests.java index a6eef6d332608..186a9284ac65e 100644 --- a/test/hotspot/jtreg/compiler/c2/gvn/ModLNodeValueTests.java +++ b/test/hotspot/jtreg/compiler/c2/gvn/ModLNodeValueTests.java @@ -35,7 +35,7 @@ /* * @test - * @bug 8356813 + * @bug 8356813 8366815 * @summary Test that Value method of ModLNode is working as expected. * @key randomness * @library /test/lib / @@ -53,6 +53,8 @@ public static void main(String[] args) { @Run(test = { "nonNegativeDividend", "nonNegativeDividendInRange", "negativeDividend", "negativeDividendInRange", + "positiveDivisor", "positiveDivisor2", + "negativeDivisor", "negativeDivisor2", "modByKnownBoundsUpper", "modByKnownBoundsUpperInRange", "modByKnownBoundsLower", "modByKnownBoundsLowerInRange", "modByKnownBoundsLimitedByDividendUpper", "modByKnownBoundsLimitedByDividendUpperInRange", @@ -78,6 +80,10 @@ public void assertResult(long x, long y) { Asserts.assertEQ(x != 0 && POS_LONG % x <= 0, nonNegativeDividendInRange(x)); Asserts.assertEQ(x != 0 && NEG_LONG % x > 0, negativeDividend(x)); Asserts.assertEQ(x != 0 && NEG_LONG % x >= 0, negativeDividendInRange(x)); + Asserts.assertEQ(x % POS_LONG >= POS_LONG, positiveDivisor(x)); + Asserts.assertEQ(x % POS_LONG <= -POS_LONG, positiveDivisor2(x)); + Asserts.assertEQ(x % NEG_LONG <= NEG_LONG, negativeDivisor(x)); + Asserts.assertEQ(x % NEG_LONG > -(NEG_LONG + 1), negativeDivisor2(x)); Asserts.assertEQ(x % (((byte) y) + 129L) > 255, modByKnownBoundsUpper(x, y)); Asserts.assertEQ(x % (((byte) y) + 129L) >= 255, modByKnownBoundsUpperInRange(x, y)); Asserts.assertEQ(x % (((byte) y) + 129L) < -255, modByKnownBoundsLower(x, y)); @@ -136,6 +142,39 @@ public boolean negativeDividendInRange(long x) { return x != 0 && NEG_LONG % x >= 0; } + @Test + @IR(failOn = {IRNode.MOD_L, IRNode.CMP_L, IRNode.AND_L, IRNode.RSHIFT_L}) + // The result is always smaller than the positive divisor. + // Constant fold to false. + public boolean positiveDivisor(long x) { + return x % POS_LONG >= POS_LONG; + } + + @Test + @IR(failOn = {IRNode.MOD_L, IRNode.CMP_L, IRNode.AND_L, IRNode.RSHIFT_L}) + // The result is always bigger than the negated positive divisor. + // Constant fold to false. + public boolean positiveDivisor2(long x) { + return x % POS_LONG <= -POS_LONG; + } + + @Test + @IR(failOn = {IRNode.MOD_L, IRNode.CMP_L, IRNode.AND_L, IRNode.RSHIFT_L}) + // The result is always smaller than the negated negative divisor with exception if MIN_VALUE. + // Constant fold to false. + public boolean negativeDivisor(long x) { + // > with + 1 to avoid -MIN_VALUE == MIN_VALUE + return x % NEG_LONG > -(NEG_LONG + 1); + } + + @Test + @IR(failOn = {IRNode.MOD_L, IRNode.CMP_L, IRNode.AND_L, IRNode.RSHIFT_L}) + // The result is always bigger than the negative divisor. + // Constant fold to false. + public boolean negativeDivisor2(long x) { + return x % NEG_LONG <= NEG_LONG; + } + @Test @IR(failOn = {IRNode.MOD_L, IRNode.CMP_L}) // The magnitude of the result is less than the divisor. From 6a3922240e8722266088405bc25717a7f075d386 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Sun, 19 Oct 2025 17:34:37 +0200 Subject: [PATCH 2/4] delay integral Div/Mod Ideal() until IGVN --- src/hotspot/share/opto/divnode.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/hotspot/share/opto/divnode.cpp b/src/hotspot/share/opto/divnode.cpp index 823745ea8e7fd..2b798d6d99102 100644 --- a/src/hotspot/share/opto/divnode.cpp +++ b/src/hotspot/share/opto/divnode.cpp @@ -541,6 +541,13 @@ Node *DivINode::Ideal(PhaseGVN *phase, bool can_reshape) { // Dividing by MININT does not optimize as a power-of-2 shift. if( i == min_jint ) return nullptr; + // Keep this node as-is for now; we want Value() and + // other optimizations checking for this node type to work + if (phase->is_IterGVN() == nullptr) { + phase->C->record_for_igvn(this); + return nullptr; + } + return transform_int_divide( phase, in(1), i ); } @@ -647,6 +654,13 @@ Node *DivLNode::Ideal( PhaseGVN *phase, bool can_reshape) { // Dividing by MINLONG does not optimize as a power-of-2 shift. if( l == min_jlong ) return nullptr; + // Keep this node as-is for now; we want Value() and + // other optimizations checking for this node type to work + if (phase->is_IterGVN() == nullptr) { + phase->C->record_for_igvn(this); + return nullptr; + } + return transform_long_divide( phase, in(1), l ); } @@ -1094,6 +1108,13 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } + // Keep this node as-is for now; we want Value() and + // other optimizations checking for this node type to work + if (phase->is_IterGVN() == nullptr) { + phase->C->record_for_igvn(this); + return nullptr; + } + // See if we are MOD'ing by 2^k or 2^k-1. if( !ti->is_con() ) return nullptr; jint con = ti->get_con(); @@ -1389,6 +1410,13 @@ Node *ModLNode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } + // Keep this node as-is for now; we want Value() and + // other optimizations checking for this node type to work + if (phase->is_IterGVN() == nullptr) { + phase->C->record_for_igvn(this); + return nullptr; + } + // See if we are MOD'ing by 2^k or 2^k-1. if( !tl->is_con() ) return nullptr; jlong con = tl->get_con(); From 6a8d842f97d334c1695a21485c2eb43dad3da2c8 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Fri, 24 Oct 2025 11:03:44 +0200 Subject: [PATCH 3/4] expand comments --- src/hotspot/share/opto/divnode.cpp | 33 ++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/hotspot/share/opto/divnode.cpp b/src/hotspot/share/opto/divnode.cpp index 2b798d6d99102..16e93a8010e51 100644 --- a/src/hotspot/share/opto/divnode.cpp +++ b/src/hotspot/share/opto/divnode.cpp @@ -541,8 +541,18 @@ Node *DivINode::Ideal(PhaseGVN *phase, bool can_reshape) { // Dividing by MININT does not optimize as a power-of-2 shift. if( i == min_jint ) return nullptr; - // Keep this node as-is for now; we want Value() and - // other optimizations checking for this node type to work + // Keep this node as-is initially; we want Value() and + // other optimizations checking for this node type to work. + // Consider the following expression: + // x / 100_000 >= 21_475, x in TypeInt::INT + // This will always be false since max_jint / 100_000 == 21_474. + // After transform_int_divide, we have a Sub node to round towards 0. + // That means we subtract -1 if the dividend is negative, and 0 otherwise. + // As the Sub node is not aware of representing a division, it overapproximates + // [-21_475, 21_474] - [-1, 0] = [-21_475, 21_475], which prevents constant folding. + // + // Less precise comparisons still work after transform_int_divide, e.g., + // comparing with >= 21_476 does not conflict with the off-by-one overapproximation. if (phase->is_IterGVN() == nullptr) { phase->C->record_for_igvn(this); return nullptr; @@ -654,8 +664,9 @@ Node *DivLNode::Ideal( PhaseGVN *phase, bool can_reshape) { // Dividing by MINLONG does not optimize as a power-of-2 shift. if( l == min_jlong ) return nullptr; - // Keep this node as-is for now; we want Value() and - // other optimizations checking for this node type to work + // Keep this node as-is initially; we want Value() and + // other optimizations checking for this node type to work. + // See DivINode::Ideal for an explanation. if (phase->is_IterGVN() == nullptr) { phase->C->record_for_igvn(this); return nullptr; @@ -1108,8 +1119,13 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } - // Keep this node as-is for now; we want Value() and - // other optimizations checking for this node type to work + // Keep this node as-is initially; we want Value() and + // other optimizations checking for this node type to work. + // Consider the following expression: + // x % 2, x in TypeInt::INT + // With ModINode::Value, we can trivially tell the resulting range is [-1,1]. + // After idealizing, we have a subtraction from x, which means without + // recognizing that as a modulo operation, we end up with a range of TypeInt::INT. if (phase->is_IterGVN() == nullptr) { phase->C->record_for_igvn(this); return nullptr; @@ -1410,8 +1426,9 @@ Node *ModLNode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } - // Keep this node as-is for now; we want Value() and - // other optimizations checking for this node type to work + // Keep this node as-is initially; we want Value() and + // other optimizations checking for this node type to work. + // See ModINode::Ideal for an explanation. if (phase->is_IterGVN() == nullptr) { phase->C->record_for_igvn(this); return nullptr; From c32bb55193c46a9445dad43941ee20843f772ee7 Mon Sep 17 00:00:00 2001 From: SirYwell Date: Thu, 30 Oct 2025 19:18:37 +0100 Subject: [PATCH 4/4] review --- src/hotspot/share/opto/divnode.cpp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/hotspot/share/opto/divnode.cpp b/src/hotspot/share/opto/divnode.cpp index 16e93a8010e51..68a86d0d37750 100644 --- a/src/hotspot/share/opto/divnode.cpp +++ b/src/hotspot/share/opto/divnode.cpp @@ -553,7 +553,7 @@ Node *DivINode::Ideal(PhaseGVN *phase, bool can_reshape) { // // Less precise comparisons still work after transform_int_divide, e.g., // comparing with >= 21_476 does not conflict with the off-by-one overapproximation. - if (phase->is_IterGVN() == nullptr) { + if (!can_reshape) { phase->C->record_for_igvn(this); return nullptr; } @@ -667,7 +667,7 @@ Node *DivLNode::Ideal( PhaseGVN *phase, bool can_reshape) { // Keep this node as-is initially; we want Value() and // other optimizations checking for this node type to work. // See DivINode::Ideal for an explanation. - if (phase->is_IterGVN() == nullptr) { + if (!can_reshape) { phase->C->record_for_igvn(this); return nullptr; } @@ -1119,6 +1119,12 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } + + // See if we are MOD'ing by 2^k or 2^k-1. + if (!ti->is_con()) { + return nullptr; + } + // Keep this node as-is initially; we want Value() and // other optimizations checking for this node type to work. // Consider the following expression: @@ -1126,13 +1132,10 @@ Node *ModINode::Ideal(PhaseGVN *phase, bool can_reshape) { // With ModINode::Value, we can trivially tell the resulting range is [-1,1]. // After idealizing, we have a subtraction from x, which means without // recognizing that as a modulo operation, we end up with a range of TypeInt::INT. - if (phase->is_IterGVN() == nullptr) { + if (!can_reshape) { phase->C->record_for_igvn(this); return nullptr; } - - // See if we are MOD'ing by 2^k or 2^k-1. - if( !ti->is_con() ) return nullptr; jint con = ti->get_con(); Node *hook = new Node(1); @@ -1426,16 +1429,19 @@ Node *ModLNode::Ideal(PhaseGVN *phase, bool can_reshape) { return this; } + // See if we are MOD'ing by 2^k or 2^k-1. + if (!tl->is_con()) { + return nullptr; + } + // Keep this node as-is initially; we want Value() and // other optimizations checking for this node type to work. // See ModINode::Ideal for an explanation. - if (phase->is_IterGVN() == nullptr) { + if (!can_reshape) { phase->C->record_for_igvn(this); return nullptr; } - // See if we are MOD'ing by 2^k or 2^k-1. - if( !tl->is_con() ) return nullptr; jlong con = tl->get_con(); Node *hook = new Node(1);