Skip to content

8347901: C2 should remove unused leaf / pure runtime calls #25760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion src/hotspot/share/opto/callnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ Node *CallNode::result_cast() {
}


void CallNode::extract_projections(CallProjections* projs, bool separate_io_proj, bool do_asserts) {
void CallNode::extract_projections(CallProjections* projs, bool separate_io_proj, bool do_asserts) const {
projs->fallthrough_proj = nullptr;
projs->fallthrough_catchproj = nullptr;
projs->fallthrough_ioproj = nullptr;
Expand Down Expand Up @@ -1303,6 +1303,53 @@ void CallLeafVectorNode::calling_convention( BasicType* sig_bt, VMRegPair *parm_


//=============================================================================
bool CallLeafPureNode::is_unused() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a quick comment why this check implies that the node is not used, i.e. what that means?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i'll need you to explain to me what is unclear at the moment. When I read the function, I see:
"A CallLeafPure is unused iff there is no output result projection."

I don't see what else to add that is not covered by "if we don't use the result, the pure call is unused", which is exactly the code. Is there any untold hypothesis lurking somewhere that I don't see? It seems to me it uses just very common concepts of C2.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call could have other uses for other projections. Why does this projection make it unused?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I was not aware that TypeFunc::Parms stands for result projection.... the name does not make it immediately apparent.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think I'd be better to comment on the declaration of the class then. Something saying that CallLeafPureNode represents calls that are pure: they only have data input and output data (and control for practical reasons for now), no exceptions, no memory, no safepoint... They can be freely be moved around, duplicated or, if the result isn't used, removed. Then that explains... a lot of what we are doing, not just is_unused.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was really just confused about the Parms. I thought that means parameters .. and not results 🤣

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually both. For functions, the parameters are starting at Parms, and the results too. Before that, it's all the special stuff: control, memory, io...

return proj_out_or_null(TypeFunc::Parms) == nullptr;
}

bool CallLeafPureNode::is_dead() const {
return proj_out_or_null(TypeFunc::Control) == nullptr;
}

/* We make a tuple of the global input state + TOP for the output values.
* We use this to delete a pure function that is not used: by replacing the call with
* such a tuple, we let output Proj's idealization pick the corresponding input of the
* pure call, so jumping over it, and effectively, removing the call from the graph.
* This avoids doing the graph surgery manually, but leave that to IGVN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This avoids doing the graph surgery manually, but leave that to IGVN
* This avoids doing the graph surgery manually, but leaves that to IGVN

* that is specialized for doing that right. We need also tuple components for output
* values of the function to respect the return arity, and in case there is a projection
* that would pick an output (which shouldn't happen at the moment).
*/
TupleNode* CallLeafPureNode::make_tuple_of_input_state_and_top_return_values(const Compile* C) const {
// Transparently propagate input state but parameters
TupleNode* tuple = TupleNode::make(
tf()->range(),
in(TypeFunc::Control),
in(TypeFunc::I_O),
in(TypeFunc::Memory),
in(TypeFunc::FramePtr),
in(TypeFunc::ReturnAdr));

// And add TOPs for the return values
for (uint i = TypeFunc::Parms; i < tf()->range()->cnt(); i++) {
tuple->set_req(i, C->top());
}

return tuple;
}

Node* CallLeafPureNode::Ideal(PhaseGVN* phase, bool can_reshape) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you make sure that this method is called from all its subclasses?

It seems to me that you just copied the code to the subclasses, rather than calling this method, am I right?

if (is_dead()) { // dead node
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (is_dead()) { // dead node
if (is_dead()) {

The comment seemed redundant. You could say who else is responsible of cleaning up the dead node though.

What would happen if the CallLeafPureNode loses its control projection, but not the other uses? I don't even know if that is possible. What do you think?

return nullptr;
}

if (can_reshape && is_unused()) {
return make_tuple_of_input_state_and_top_return_values(phase->C);
}

return CallRuntimeNode::Ideal(phase, can_reshape);
}

#ifndef PRODUCT
void CallLeafNode::dump_spec(outputStream *st) const {
st->print("# ");
Expand Down
18 changes: 17 additions & 1 deletion src/hotspot/share/opto/callnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,7 @@ class CallNode : public SafePointNode {
// Collect all the interesting edges from a call for use in
// replacing the call by something else. Used by macro expansion
// and the late inlining support.
void extract_projections(CallProjections* projs, bool separate_io_proj, bool do_asserts = true);
void extract_projections(CallProjections* projs, bool separate_io_proj, bool do_asserts = true) const;

virtual uint match_edge(uint idx) const;

Expand Down Expand Up @@ -913,6 +913,22 @@ class CallLeafNode : public CallRuntimeNode {
#endif
};

class CallLeafPureNode : public CallLeafNode {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need a short description about what this node is for. What are the assumptions about it?

protected:
bool is_unused() const;
bool is_dead() const;
TupleNode* make_tuple_of_input_state_and_top_return_values(const Compile* C) const;

public:
CallLeafPureNode(const TypeFunc* tf, address addr, const char* name,
const TypePtr* adr_type)
: CallLeafNode(tf, addr, name, adr_type) {
init_class_id(Class_CallLeafPure);
}
int Opcode() const override;
Node* Ideal(PhaseGVN* phase, bool can_reshape) override;
};

//------------------------------CallLeafNoFPNode-------------------------------
// CallLeafNode, not using floating point or using it in the same manner as
// the generated code
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/classes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ macro(CallDynamicJava)
macro(CallJava)
macro(CallLeaf)
macro(CallLeafNoFP)
macro(CallLeafPure)
macro(CallLeafVector)
macro(CallRuntime)
macro(CallStaticJava)
Expand Down Expand Up @@ -372,6 +373,7 @@ macro(SubI)
macro(SubL)
macro(TailCall)
macro(TailJump)
macro(Tuple)
macro(MacroLogicV)
macro(ThreadLocal)
macro(Unlock)
Expand Down
19 changes: 19 additions & 0 deletions src/hotspot/share/opto/compile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3298,6 +3298,25 @@ void Compile::final_graph_reshaping_main_switch(Node* n, Final_Reshape_Counts& f
case Op_Opaque1: // Remove Opaque Nodes before matching
n->subsume_by(n->in(1), this);
break;
case Op_CallLeafPure: {
// If the pure call is not supported, then lower to a CallLeaf.
if (!Matcher::match_rule_supported(Op_CallLeafPure)) {
CallNode* call = n->as_Call();
CallNode* new_call = new CallLeafNode(call->tf(), call->entry_point(),
call->_name, TypeRawPtr::BOTTOM);
new_call->init_req(TypeFunc::Control, call->in(TypeFunc::Control));
new_call->init_req(TypeFunc::I_O, C->top());
new_call->init_req(TypeFunc::Memory, C->top());
new_call->init_req(TypeFunc::ReturnAdr, C->top());
new_call->init_req(TypeFunc::FramePtr, C->top());
for (unsigned int i = TypeFunc::Parms; i < call->tf()->domain()->cnt(); i++) {
new_call->init_req(i, call->in(i));
}
n->subsume_by(new_call, this);
}
frc.inc_call_count();
break;
}
case Op_CallStaticJava:
case Op_CallJava:
case Op_CallDynamicJava:
Expand Down
40 changes: 15 additions & 25 deletions src/hotspot/share/opto/divnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@

#include <math.h>

ModFloatingNode::ModFloatingNode(Compile* C, const TypeFunc* tf, const char* name) : CallLeafNode(tf, nullptr, name, TypeRawPtr::BOTTOM) {
ModFloatingNode::ModFloatingNode(Compile* C, const TypeFunc* tf, address addr, const char* name) : CallLeafPureNode(tf, addr, name, TypeRawPtr::BOTTOM) {
add_flag(Flag_is_macro);
C->add_macro_node(this);
}

ModDNode::ModDNode(Compile* C, Node* a, Node* b) : ModFloatingNode(C, OptoRuntime::Math_DD_D_Type(), "drem") {
ModDNode::ModDNode(Compile* C, Node* a, Node* b) : ModFloatingNode(C, OptoRuntime::Math_DD_D_Type(), CAST_FROM_FN_PTR(address, SharedRuntime::drem), "drem") {
init_req(TypeFunc::Parms + 0, a);
init_req(TypeFunc::Parms + 1, C->top());
init_req(TypeFunc::Parms + 2, b);
init_req(TypeFunc::Parms + 3, C->top());
}

ModFNode::ModFNode(Compile* C, Node* a, Node* b) : ModFloatingNode(C, OptoRuntime::modf_Type(), "frem") {
ModFNode::ModFNode(Compile* C, Node* a, Node* b) : ModFloatingNode(C, OptoRuntime::modf_Type(), CAST_FROM_FN_PTR(address, SharedRuntime::frem), "frem") {
init_req(TypeFunc::Parms + 0, a);
init_req(TypeFunc::Parms + 1, b);
}
Expand Down Expand Up @@ -1520,12 +1520,14 @@ Node* ModFNode::Ideal(PhaseGVN* phase, bool can_reshape) {
if (!can_reshape) {
return nullptr;
}
Comment on lines 1520 to 1522
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this prevent us from doing the make_tuple_of_input_state_and_top_return_values trick? Because it seems to me that we do not need to reshape the node for that, right? Maybe you should reorder things for that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, you should probably call CallLeafPureNode::Ideal instead of duplicating its logic here and in other subclasses.

if (is_dead()) { // dead node
return nullptr;
}

PhaseIterGVN* igvn = phase->is_IterGVN();

bool result_is_unused = proj_out_or_null(TypeFunc::Parms) == nullptr;
bool not_dead = proj_out_or_null(TypeFunc::Control) != nullptr;
if (result_is_unused && not_dead) {
return replace_with_con(igvn, TypeF::make(0.));
if (is_unused()) {
return make_tuple_of_input_state_and_top_return_values(igvn->C);
}

// Either input is TOP ==> the result is TOP
Expand Down Expand Up @@ -1572,12 +1574,14 @@ Node* ModDNode::Ideal(PhaseGVN* phase, bool can_reshape) {
if (!can_reshape) {
return nullptr;
}
if (is_dead()) { // dead node
return nullptr;
}

PhaseIterGVN* igvn = phase->is_IterGVN();

bool result_is_unused = proj_out_or_null(TypeFunc::Parms) == nullptr;
bool not_dead = proj_out_or_null(TypeFunc::Control) != nullptr;
if (result_is_unused && not_dead) {
return replace_with_con(igvn, TypeD::make(0.));
if (is_unused()) {
return make_tuple_of_input_state_and_top_return_values(igvn->C);
}

// Either input is TOP ==> the result is TOP
Expand Down Expand Up @@ -1626,20 +1630,6 @@ Node* ModFloatingNode::replace_with_con(PhaseIterGVN* phase, const Type* con) {
CallProjections projs;
extract_projections(&projs, false, false);
phase->replace_node(projs.fallthrough_proj, in(TypeFunc::Control));
if (projs.fallthrough_catchproj != nullptr) {
phase->replace_node(projs.fallthrough_catchproj, in(TypeFunc::Control));
}
if (projs.fallthrough_memproj != nullptr) {
phase->replace_node(projs.fallthrough_memproj, in(TypeFunc::Memory));
}
if (projs.catchall_memproj != nullptr) {
phase->replace_node(projs.catchall_memproj, C->top());
}
if (projs.fallthrough_ioproj != nullptr) {
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) {
phase->replace_node(projs.resproj, con_node);
}
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/divnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,12 @@ class ModLNode : public Node {
};

// Base class for float and double modulus
class ModFloatingNode : public CallLeafNode {
class ModFloatingNode : public CallLeafPureNode {
protected:
Node* replace_with_con(PhaseIterGVN* phase, const Type* con);

public:
ModFloatingNode(Compile* C, const TypeFunc* tf, const char *name);
ModFloatingNode(Compile* C, const TypeFunc* tf, address addr, const char* name);
};

// Float Modulus
Expand Down
29 changes: 21 additions & 8 deletions src/hotspot/share/opto/graphKit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1880,14 +1880,20 @@ Node* GraphKit::set_results_for_java_call(CallJavaNode* call, bool separate_io_p
// after the call, if this call has restricted memory effects.
Node* GraphKit::set_predefined_input_for_runtime_call(SafePointNode* call, Node* narrow_mem) {
// Set fixed predefined input arguments
Node* memory = reset_memory();
Node* m = narrow_mem == nullptr ? memory : narrow_mem;
call->init_req( TypeFunc::Control, control() );
call->init_req( TypeFunc::I_O, top() ); // does no i/o
call->init_req( TypeFunc::Memory, m ); // may gc ptrs
call->init_req( TypeFunc::FramePtr, frameptr() );
call->init_req( TypeFunc::ReturnAdr, top() );
return memory;
call->init_req(TypeFunc::Control, control());
call->init_req(TypeFunc::I_O, top()); // does no i/o
call->init_req(TypeFunc::ReturnAdr, top());
if (call->is_CallLeafPure()) {
call->init_req(TypeFunc::Memory, top());
call->init_req(TypeFunc::FramePtr, top());
return nullptr;
} else {
Node* memory = reset_memory();
Node* m = narrow_mem == nullptr ? memory : narrow_mem;
call->init_req(TypeFunc::Memory, m); // may gc ptrs
call->init_req(TypeFunc::FramePtr, frameptr());
return memory;
}
}

//-------------------set_predefined_output_for_runtime_call--------------------
Expand All @@ -1905,6 +1911,11 @@ void GraphKit::set_predefined_output_for_runtime_call(Node* call,
const TypePtr* hook_mem) {
// no i/o
set_control(_gvn.transform( new ProjNode(call,TypeFunc::Control) ));
if (call->is_CallLeafPure()) {
// Pure function have only control (for now) and data output, in particular
// the don't touch the memory, so we don't want a memory proj that is set after.
return;
}
if (keep_mem) {
// First clone the existing memory state
set_all_memory(keep_mem);
Expand Down Expand Up @@ -2491,6 +2502,8 @@ Node* GraphKit::make_runtime_call(int flags,
} else if (flags & RC_VECTOR){
uint num_bits = call_type->range()->field_at(TypeFunc::Parms)->is_vect()->length_in_bytes() * BitsPerByte;
call = new CallLeafVectorNode(call_type, call_addr, call_name, adr_type, num_bits);
} else if (flags & RC_PURE) {
call = new CallLeafPureNode(call_type, call_addr, call_name, adr_type);
} else {
call = new CallLeafNode(call_type, call_addr, call_name, adr_type);
}
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/opto/graphKit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ class GraphKit : public Phase {
RC_NARROW_MEM = 16, // input memory is same as output
RC_UNCOMMON = 32, // freq. expected to be like uncommon trap
RC_VECTOR = 64, // CallLeafVectorNode
RC_PURE = 128, // CallLeaf is pure
RC_LEAF = 0 // null value: no flags set
};

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1797,7 +1797,7 @@ bool LibraryCallKit::runtime_math(const TypeFunc* call_type, address funcAddr, c
Node* b = (call_type == OptoRuntime::Math_DD_D_Type()) ? argument(2) : nullptr;

const TypePtr* no_memory_effects = nullptr;
Node* trig = make_runtime_call(RC_LEAF, call_type, funcAddr, funcName,
Node* trig = make_runtime_call(RC_LEAF | RC_PURE, call_type, funcAddr, funcName,
no_memory_effects,
a, top(), b, b ? top() : nullptr);
Node* value = _gvn.transform(new ProjNode(trig, TypeFunc::Parms+0));
Expand Down
15 changes: 6 additions & 9 deletions src/hotspot/share/opto/macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2597,17 +2597,14 @@ bool PhaseMacroExpand::expand_macro_nodes() {
switch (n->Opcode()) {
case Op_ModD:
case Op_ModF: {
bool is_drem = n->Opcode() == Op_ModD;
CallNode* mod_macro = n->as_Call();
CallNode* call = new CallLeafNode(mod_macro->tf(),
is_drem ? CAST_FROM_FN_PTR(address, SharedRuntime::drem)
: CAST_FROM_FN_PTR(address, SharedRuntime::frem),
is_drem ? "drem" : "frem", TypeRawPtr::BOTTOM);
CallNode* call = new CallLeafPureNode(mod_macro->tf(), mod_macro->entry_point(),
mod_macro->_name, TypeRawPtr::BOTTOM);
call->init_req(TypeFunc::Control, mod_macro->in(TypeFunc::Control));
call->init_req(TypeFunc::I_O, mod_macro->in(TypeFunc::I_O));
call->init_req(TypeFunc::Memory, mod_macro->in(TypeFunc::Memory));
call->init_req(TypeFunc::ReturnAdr, mod_macro->in(TypeFunc::ReturnAdr));
call->init_req(TypeFunc::FramePtr, mod_macro->in(TypeFunc::FramePtr));
call->init_req(TypeFunc::I_O, C->top());
call->init_req(TypeFunc::Memory, C->top());
call->init_req(TypeFunc::ReturnAdr, C->top());
call->init_req(TypeFunc::FramePtr, C->top());
for (unsigned int i = 0; i < mod_macro->tf()->domain()->cnt() - TypeFunc::Parms; i++) {
call->init_req(TypeFunc::Parms + i, mod_macro->in(TypeFunc::Parms + i));
}
Expand Down
13 changes: 13 additions & 0 deletions src/hotspot/share/opto/multnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ const TypePtr *ProjNode::adr_type() const {
if (bottom_type() == Type::MEMORY) {
// in(0) might be a narrow MemBar; otherwise we will report TypePtr::BOTTOM
Node* ctrl = in(0);
if (ctrl->Opcode() == Op_Tuple) {
// Jumping over Tuples: the i-th projection of a Tuple is the i-th input of the Tuple.
ctrl = ctrl->in(_con);
}
Comment on lines +123 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to special-case this here? Why does the ProjNode::Identity not suffice? Are there potentially other locations where we now would need this special logic?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good question. That is something I picked from Vladimir's implementation and it seemed legitimate. But now you say it, is it needed? Not sure. I'm trying to find that out. Would ::Identity be enough? It's tempting to say so, right! I'd say it can be not enough if we need adr_type before idealizing the ProjNode (no idea if that happens). Is there any other places to adapt? One could think so, but actually, I can't find such an example. Other methods of ProjNode for instance rely on the type of the input (which is correctly handled in TupleNode), and so should already work fine.

I'm trying to understand what happens if we don't have that. But maybe @iwanowww would have some helpful insight?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember all the details now, but there were some problems when ProjNode::adr_type() encounters TupleNode.

if (ctrl == nullptr) return nullptr; // node is dead
const TypePtr* adr_type = ctrl->adr_type();
#ifdef ASSERT
Expand Down Expand Up @@ -163,6 +167,15 @@ void ProjNode::check_con() const {
assert(_con < t->is_tuple()->cnt(), "ProjNode::_con must be in range");
}

//------------------------------Identity---------------------------------------
Node* ProjNode::Identity(PhaseGVN* phase) {
if (in(0) != nullptr && in(0)->Opcode() == Op_Tuple) {
// Jumping over Tuples: the i-th projection of a Tuple is the i-th input of the Tuple.
return in(0)->in(_con);
}
return this;
}

//------------------------------Value------------------------------------------
const Type* ProjNode::Value(PhaseGVN* phase) const {
if (in(0) == nullptr) return Type::TOP;
Expand Down
Loading