RFR: 8321823: Remove redundant PhaseGVN transform and transform_no_reclaim
Christian Hagedorn
chagedorn at openjdk.org
Tue Dec 12 08:22:34 UTC 2023
On Tue, 12 Dec 2023 00:37:19 GMT, Joshua Cao <duke at openjdk.org> wrote:
> `PhaseGVN::transform` is just a one line wrapper around `PhaseGVN::transform_no_reclaim`. Looking at the history, they had different functionality in 2008, but since have become the same thing. We prefer to keep `PhaseGVN::transform` because it has hundreds of callsites, while the other only has a few callsites that are shown in the PR.
>
> Passes tier1 locally on my Linux machine.
The title of the RFE is a little bit misleading as it suggests that both are redundant. Maybe you can change that into just mentioning that `PhaseGVN::transform_no_reclaim()` is redundant.
Otherwise, looks good!
src/hotspot/share/opto/phaseX.cpp line 676:
> 674: // Return a node which computes the same function as this node, but
> 675: // in a faster or cheaper fashion.
> 676: Node *PhaseGVN::transform(Node *n) {
While at it, you could also fix the asterisk positions:
Suggestion:
Node* PhaseGVN::transform(Node* n) {
src/hotspot/share/opto/phaseX.hpp line 418:
> 416: // Return a node which computes the same function as this node, but
> 417: // in a faster or cheaper fashion.
> 418: Node *transform(Node *n);
Same here and maybe add a new line afterward to separate it better from `record_for_igvn()`.
Suggestion:
Node* transform(Node* n);
-------------
Marked as reviewed by chagedorn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17071#pullrequestreview-1776879595
PR Review Comment: https://git.openjdk.org/jdk/pull/17071#discussion_r1423604333
PR Review Comment: https://git.openjdk.org/jdk/pull/17071#discussion_r1423606056
More information about the hotspot-compiler-dev
mailing list