RFR: 8322490: CastNode constructors accepts control node as input

Christian Hagedorn chagedorn at openjdk.org
Wed Dec 20 07:44:46 UTC 2023


On Tue, 19 Dec 2023 20:27:06 GMT, Joshua Cao <duke at openjdk.org> wrote:

> It is a common pattern to have:
> 
> 
> Node* n = new CastNode(...);
> n->set_req(control_node);
> 
> 
> We can modify the constructor to set the control node. It makes the code a little tidier.
> 
> Passes tier1 locally on my Linux machine

I think that's a good idea but you've missed some cases of creating new `CastIINodes` that could use the control-based constructor.

Example (there are more cases when you search for "new CastIINode"):
https://github.com/openjdk/jdk/blob/f7dc257a206d3104d6d24c2079ef1fe349368c49/src/hotspot/share/opto/loopTransform.cpp#L3164-L3166

src/hotspot/share/opto/castnode.cpp line 126:

> 124: }
> 125: 
> 126: Node* ConstraintCastNode::make_cast(int opcode, Node* c, Node* n, const Type* t, DependencyType dependency,

I'm wondering if this method is still worth keeping as it would simply replace a "new CastXXNode" line which is just as expressive/readable. As far as I can see, it's currently only used with statically known Opcodes. So, we could replace them.

-------------

Changes requested by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17162#pullrequestreview-1790330182
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1432357733


More information about the hotspot-compiler-dev mailing list