8349523: Unused runtime calls to drem/frem should be removed

Reviewed-by: thartmann, kvn, chagedorn
This commit is contained in:
Marc Chevalier 2025-03-03 09:32:54 +00:00
parent b054d24df5
commit 4109c73a78
9 changed files with 253 additions and 17 deletions

View File

@ -436,6 +436,9 @@ void Compile::disconnect_useless_nodes(Unique_Node_List& useful, Unique_Node_Lis
n->raw_del_out(j);
--j;
--max;
if (child->is_data_proj_of_pure_function(n)) {
worklist.push(n);
}
}
}
if (n->outcnt() == 1 && n->has_special_unique_user()) {

View File

@ -1515,6 +1515,12 @@ Node* ModFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
if (!can_reshape) {
return nullptr;
}
PhaseIterGVN* igvn = phase->is_IterGVN();
bool result_is_unused = proj_out_or_null(TypeFunc::Parms) == nullptr;
if (result_is_unused) {
return replace_with_con(igvn, TypeF::make(0.));
}
// Either input is TOP ==> the result is TOP
const Type* t1 = phase->type(dividend());
@ -1535,10 +1541,10 @@ Node* ModFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
// If either is a NaN, return an input NaN
if (g_isnan(f1)) {
return replace_with_con(phase, t1);
return replace_with_con(igvn, t1);
}
if (g_isnan(f2)) {
return replace_with_con(phase, t2);
return replace_with_con(igvn, t2);
}
// If an operand is infinity or the divisor is +/- zero, punt.
@ -1553,13 +1559,19 @@ Node* ModFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
xr ^= min_jint;
}
return replace_with_con(phase, TypeF::make(jfloat_cast(xr)));
return replace_with_con(igvn, TypeF::make(jfloat_cast(xr)));
}
Node* ModDNode::Ideal(PhaseGVN* phase, bool can_reshape) {
if (!can_reshape) {
return nullptr;
}
PhaseIterGVN* igvn = phase->is_IterGVN();
bool result_is_unused = proj_out_or_null(TypeFunc::Parms) == nullptr;
if (result_is_unused) {
return replace_with_con(igvn, TypeD::make(0.));
}
// Either input is TOP ==> the result is TOP
const Type* t1 = phase->type(dividend());
@ -1580,10 +1592,10 @@ Node* ModDNode::Ideal(PhaseGVN* phase, bool can_reshape) {
// If either is a NaN, return an input NaN
if (g_isnan(f1)) {
return replace_with_con(phase, t1);
return replace_with_con(igvn, t1);
}
if (g_isnan(f2)) {
return replace_with_con(phase, t2);
return replace_with_con(igvn, t2);
}
// If an operand is infinity or the divisor is +/- zero, punt.
@ -1598,33 +1610,33 @@ Node* ModDNode::Ideal(PhaseGVN* phase, bool can_reshape) {
xr ^= min_jlong;
}
return replace_with_con(phase, TypeD::make(jdouble_cast(xr)));
return replace_with_con(igvn, TypeD::make(jdouble_cast(xr)));
}
Node* ModFloatingNode::replace_with_con(PhaseGVN* phase, const Type* con) {
Node* ModFloatingNode::replace_with_con(PhaseIterGVN* phase, const Type* con) {
Compile* C = phase->C;
Node* con_node = phase->makecon(con);
CallProjections projs;
extract_projections(&projs, false, false);
C->gvn_replace_by(projs.fallthrough_proj, in(TypeFunc::Control));
phase->replace_node(projs.fallthrough_proj, in(TypeFunc::Control));
if (projs.fallthrough_catchproj != nullptr) {
C->gvn_replace_by(projs.fallthrough_catchproj, in(TypeFunc::Control));
phase->replace_node(projs.fallthrough_catchproj, in(TypeFunc::Control));
}
if (projs.fallthrough_memproj != nullptr) {
C->gvn_replace_by(projs.fallthrough_memproj, in(TypeFunc::Memory));
phase->replace_node(projs.fallthrough_memproj, in(TypeFunc::Memory));
}
if (projs.catchall_memproj != nullptr) {
C->gvn_replace_by(projs.catchall_memproj, C->top());
phase->replace_node(projs.catchall_memproj, C->top());
}
if (projs.fallthrough_ioproj != nullptr) {
C->gvn_replace_by(projs.fallthrough_ioproj, in(TypeFunc::I_O));
phase->replace_node(projs.fallthrough_ioproj, in(TypeFunc::I_O));
}
assert(projs.catchall_ioproj == nullptr, "no exceptions from floating mod");
assert(projs.catchall_catchproj == nullptr, "no exceptions from floating mod");
if (projs.resproj != nullptr) {
C->gvn_replace_by(projs.resproj, con_node);
phase->replace_node(projs.resproj, con_node);
}
C->gvn_replace_by(this, C->top());
phase->replace_node(this, C->top());
C->remove_macro_node(this);
disconnect_inputs(C);
return nullptr;

View File

@ -158,7 +158,7 @@ public:
// Base class for float and double modulus
class ModFloatingNode : public CallLeafNode {
protected:
Node* replace_with_con(PhaseGVN* phase, const Type* con);
Node* replace_with_con(PhaseIterGVN* phase, const Type* con);
public:
ModFloatingNode(Compile* C, const TypeFunc* tf, const char *name);

View File

@ -1452,6 +1452,8 @@ static void kill_dead_code( Node *dead, PhaseIterGVN *igvn ) {
// The restriction (outcnt() <= 2) is the same as in set_req_X()
// and remove_globally_dead_node().
igvn->add_users_to_worklist( n );
} else if (dead->is_data_proj_of_pure_function(n)) {
igvn->_worklist.push(n);
} else {
BarrierSet::barrier_set()->barrier_set_c2()->enqueue_useful_gc_barrier(igvn, n);
}
@ -2931,6 +2933,25 @@ bool Node::is_dead_loop_safe() const {
bool Node::is_div_or_mod(BasicType bt) const { return Opcode() == Op_Div(bt) || Opcode() == Op_Mod(bt) ||
Opcode() == Op_UDiv(bt) || Opcode() == Op_UMod(bt); }
bool Node::is_pure_function() const {
switch (Opcode()) {
case Op_ModD:
case Op_ModF:
return true;
default:
return false;
}
}
// `maybe_pure_function` is assumed to be the input of `this`. This is a bit redundant,
// but we already have and need maybe_pure_function in all the call sites, so
// it makes it obvious that the `maybe_pure_function` is the same node as in the caller,
// while it takes more thinking to realize that a locally computed in(0) must be equal to
// the local in the caller.
bool Node::is_data_proj_of_pure_function(const Node* maybe_pure_function) const {
return Opcode() == Op_Proj && as_Proj()->_con == TypeFunc::Parms && maybe_pure_function->is_pure_function();
}
//=============================================================================
//------------------------------yank-------------------------------------------
// Find and remove

View File

@ -1284,6 +1284,10 @@ public:
bool is_div_or_mod(BasicType bt) const;
bool is_pure_function() const;
bool is_data_proj_of_pure_function(const Node* maybe_pure_function) const;
//----------------- Printing, etc
#ifndef PRODUCT
public:

View File

@ -1342,6 +1342,8 @@ void PhaseIterGVN::remove_globally_dead_node( Node *dead ) {
}
assert(!(i < imax), "sanity");
}
} else if (dead->is_data_proj_of_pure_function(in)) {
_worklist.push(in);
} else {
BarrierSet::barrier_set()->barrier_set_c2()->enqueue_useful_gc_barrier(this, in);
}

View File

@ -41,7 +41,13 @@ public class ModDNodeTests {
TestFramework.run();
}
@Run(test = {"constant", "notConstant", "veryNotConstant"})
@Run(test = {"constant", "notConstant", "veryNotConstant",
"unusedResult",
"repeatedlyUnused",
"unusedResultAfterLoopOpt1",
"unusedResultAfterLoopOpt2",
"unusedResultAfterLoopOpt3",
})
public void runMethod() {
Asserts.assertEQ(constant(), q % 72.0d % 30.0d);
Asserts.assertEQ(alsoConstant(), q % 31.432d);
@ -49,6 +55,11 @@ public class ModDNodeTests {
Asserts.assertTrue(Double.isNaN(nanRightConstant()));
Asserts.assertEQ(notConstant(37.5d), 37.5d % 32.0d);
Asserts.assertEQ(veryNotConstant(531.25d, 14.5d), 531.25d % 32.0d % 14.5d);
unusedResult(1.1d, 2.2d);
repeatedlyUnused(1.1d, 2.2d);
Asserts.assertEQ(unusedResultAfterLoopOpt1(1.1d, 2.2d), 0.d);
Asserts.assertEQ(unusedResultAfterLoopOpt2(1.1d, 2.2d), 0.d);
Asserts.assertEQ(unusedResultAfterLoopOpt3(1.1d, 2.2d), 0.d);
}
@Test
@ -114,4 +125,80 @@ public class ModDNodeTests {
public double veryNotConstant(double x, double y) {
return x % 32.0d % y;
}
@Test
@IR(failOn = IRNode.MOD_D, phase = CompilePhase.ITER_GVN1)
@IR(counts = {IRNode.MOD_D, "1"}, phase = CompilePhase.AFTER_PARSING)
public void unusedResult(double x, double y) {
double unused = x % y;
}
@Test
@IR(failOn = IRNode.MOD_D, phase = CompilePhase.ITER_GVN1)
@IR(counts = {IRNode.MOD_D, "1"}, phase = CompilePhase.AFTER_PARSING)
public void repeatedlyUnused(double x, double y) {
double unused = 1.d;
for (int i = 0; i < 100_000; i++) {
unused = x % y;
}
}
// The difference between unusedResultAfterLoopOpt1 and unusedResultAfterLoopOpt2
// is that they exercise a slightly different reason why the node is being removed,
// and thus a different execution path. In unusedResultAfterLoopOpt1 the modulo is
// used in the traps of the parse predicates. In unusedResultAfterLoopOpt2, it is not.
@Test
@IR(counts = {IRNode.MOD_D, "1"}, phase = CompilePhase.ITER_GVN2)
@IR(failOn = IRNode.MOD_D, phase = CompilePhase.BEFORE_MACRO_EXPANSION)
public double unusedResultAfterLoopOpt1(double x, double y) {
double unused = x % y;
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
if (b == 78) { // dead
return unused;
}
return 0.d;
}
@Test
@IR(counts = {IRNode.MOD_D, "1"}, phase = CompilePhase.AFTER_CLOOPS)
@IR(failOn = IRNode.MOD_D, phase = CompilePhase.PHASEIDEALLOOP1)
public double unusedResultAfterLoopOpt2(double x, double y) {
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
double unused = x % y;
if (b == 78) { // dead
return unused;
}
return 0.d;
}
@Test
@IR(counts = {IRNode.MOD_D, "2"}, phase = CompilePhase.AFTER_CLOOPS)
@IR(failOn = IRNode.MOD_D, phase = CompilePhase.PHASEIDEALLOOP1)
public double unusedResultAfterLoopOpt3(double x, double y) {
double unused = x % y;
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
int other = (b - 77) * (int)(x % y % 1.d);
return (double)other;
}
}

View File

@ -41,7 +41,13 @@ public class ModFNodeTests {
TestFramework.run();
}
@Run(test = {"constant", "notConstant", "veryNotConstant"})
@Run(test = {"constant", "notConstant", "veryNotConstant",
"unusedResult",
"repeatedlyUnused",
"unusedResultAfterLoopOpt1",
"unusedResultAfterLoopOpt2",
"unusedResultAfterLoopOpt3",
})
public void runMethod() {
Asserts.assertEQ(constant(), q % 72.0f % 30.0f);
Asserts.assertEQ(alsoConstant(), q % 31.432f);
@ -49,6 +55,11 @@ public class ModFNodeTests {
Asserts.assertTrue(Float.isNaN(nanRightConstant()));
Asserts.assertEQ(notConstant(37.5f), 37.5f % 32.0f);
Asserts.assertEQ(veryNotConstant(531.25f, 14.5f), 531.25f % 32.0f % 14.5f);
unusedResult(1.1f, 2.2f);
repeatedlyUnused(1.1f, 2.2f);
Asserts.assertEQ(unusedResultAfterLoopOpt1(1.1f, 2.2f), 0.f);
Asserts.assertEQ(unusedResultAfterLoopOpt2(1.1f, 2.2f), 0.f);
Asserts.assertEQ(unusedResultAfterLoopOpt3(1.1f, 2.2f), 0.f);
}
@Test
@ -114,4 +125,80 @@ public class ModFNodeTests {
public float veryNotConstant(float x, float y) {
return x % 32.0f % y;
}
@Test
@IR(failOn = IRNode.MOD_F, phase = CompilePhase.ITER_GVN1)
@IR(counts = {IRNode.MOD_F, "1"}, phase = CompilePhase.AFTER_PARSING)
public void unusedResult(float x, float y) {
float unused = x % y;
}
@Test
@IR(failOn = IRNode.MOD_F, phase = CompilePhase.ITER_GVN1)
@IR(counts = {IRNode.MOD_F, "1"}, phase = CompilePhase.AFTER_PARSING)
public void repeatedlyUnused(float x, float y) {
float unused = 1.f;
for (int i = 0; i < 100_000; i++) {
unused = x % y;
}
}
// The difference between unusedResultAfterLoopOpt1 and unusedResultAfterLoopOpt2
// is that they exercise a slightly different reason why the node is being removed,
// and thus a different execution path. In unusedResultAfterLoopOpt1 the modulo is
// used in the traps of the parse predicates. In unusedResultAfterLoopOpt2, it is not.
@Test
@IR(counts = {IRNode.MOD_F, "1"}, phase = CompilePhase.ITER_GVN2)
@IR(failOn = IRNode.MOD_F, phase = CompilePhase.BEFORE_MACRO_EXPANSION)
public float unusedResultAfterLoopOpt1(float x, float y) {
float unused = x % y;
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
if (b == 78) { // dead
return unused;
}
return 0.f;
}
@Test
@IR(counts = {IRNode.MOD_F, "1"}, phase = CompilePhase.AFTER_CLOOPS)
@IR(failOn = IRNode.MOD_F, phase = CompilePhase.PHASEIDEALLOOP1)
public float unusedResultAfterLoopOpt2(float x, float y) {
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
float unused = x % y;
if (b == 78) { // dead
return unused;
}
return 0.f;
}
@Test
@IR(counts = {IRNode.MOD_F, "2"}, phase = CompilePhase.AFTER_CLOOPS)
@IR(failOn = IRNode.MOD_F, phase = CompilePhase.PHASEIDEALLOOP1)
public float unusedResultAfterLoopOpt3(float x, float y) {
float unused = x % y;
int a = 77;
int b = 0;
do {
a--;
b++;
} while (a > 0);
int other = (b - 77) * (int)(x % y % 1.f);
return (float)other;
}
}

View File

@ -2531,6 +2531,16 @@ public class IRNode {
machOnlyNameRegex(X86_CMOVEL_IMM01UCF, "cmovL_imm_01UCF");
}
public static final String MOD_F = PREFIX + "MOD_F" + POSTFIX;
static {
macroNodes(MOD_F, "ModF");
}
public static final String MOD_D = PREFIX + "MOD_D" + POSTFIX;
static {
macroNodes(MOD_D, "ModD");
}
/*
* Utility methods to set up IR_NODE_MAPPINGS.
*/
@ -2576,6 +2586,16 @@ public class IRNode {
IR_NODE_MAPPINGS.put(irNode, entry);
}
/**
* Apply {@code regex} on all ideal graph phases up to and including {@link CompilePhase#BEFORE_MACRO_EXPANSION}.
*/
private static void macroNodes(String irNodePlaceholder, String irNodeRegex) {
String regex = START + irNodeRegex + MID + END;
IR_NODE_MAPPINGS.put(irNodePlaceholder, new SinglePhaseRangeEntry(CompilePhase.BEFORE_MACRO_EXPANSION, regex,
CompilePhase.BEFORE_STRINGOPTS,
CompilePhase.BEFORE_MACRO_EXPANSION));
}
private static void callOfNodes(String irNodePlaceholder, String callRegex) {
String regex = START + callRegex + MID + IS_REPLACED + " " + END;
IR_NODE_MAPPINGS.put(irNodePlaceholder, new RegexTypeEntry(RegexType.IDEAL_INDEPENDENT, regex));