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