Both CCP and IGVN need to notify outputs if an input has been updated, so that the optimization can propagate down. But we currently have two different implementations for that:
PhaseCCP::push_child_nodes_to_worklist
PhaseIterGVN::add_users_to_worklist
To some degree this makes sense, since there are slight variations. And we want to notify as little as possible. On the other hand, the code is a bit messy, and just grows as we discover more bugs. Plus, usually people will miss that they have to change one of the two, and probably are not aware that they need to change both.
Suggestion: refactor by implementing it as a virtual method of Node, and have phase as parameter. Chose behaviour based on phase: IGVN will probably notify more than CCP (subset, because only runs Value, not Ideal and Identity). The virtual member function makes it more OOP, and removes all of the type checks we are currently doing.
PhaseCCP::push_child_nodes_to_worklist
PhaseIterGVN::add_users_to_worklist
To some degree this makes sense, since there are slight variations. And we want to notify as little as possible. On the other hand, the code is a bit messy, and just grows as we discover more bugs. Plus, usually people will miss that they have to change one of the two, and probably are not aware that they need to change both.
Suggestion: refactor by implementing it as a virtual method of Node, and have phase as parameter. Chose behaviour based on phase: IGVN will probably notify more than CCP (subset, because only runs Value, not Ideal and Identity). The virtual member function makes it more OOP, and removes all of the type checks we are currently doing.
- relates to
-
JDK-8298951 Umbrella: improve CCP and IGVN verification
- Open
-
JDK-8257197 Add additional verification code to PhaseCCP
- Resolved