RFR: 8322490: cleanup CastNode construction [v4]

Christian Hagedorn chagedorn at openjdk.org
Fri Dec 22 13:17:51 UTC 2023


On Fri, 22 Dec 2023 00:46:04 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
>
> Joshua Cao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup make_cast functions

Otherwise, the update looks good! You can directly integrate/get it sponsored afterward.

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

> 124: }
> 125: 
> 126: Node* ConstraintCastNode::make_cast_for_basic_type(Node* c, Node *n, const Type *t, DependencyType dependency, BasicType bt) {

While at it, you can also fix the asterisk positions:
Suggestion:

Node* ConstraintCastNode::make_cast_for_basic_type(Node* c, Node* n, const Type* t, DependencyType dependency, BasicType bt) {

src/hotspot/share/opto/castnode.hpp line 72:

> 70:   bool carry_dependency() const { return _dependency != RegularDependency; }
> 71:   TypeNode* dominating_cast(PhaseGVN* gvn, PhaseTransform* pt) const;
> 72:   static Node* make_cast_for_basic_type(Node* c, Node *n, const Type *t, DependencyType dependency, BasicType bt);

Suggestion:

  static Node* make_cast_for_basic_type(Node* c, Node* n, const Type* t, DependencyType dependency, BasicType bt);

src/hotspot/share/opto/library_call.cpp line 1142:

> 1140:   // length is now known positive, add a cast node to make this explicit
> 1141:   jlong upper_bound = _gvn.type(length)->is_integer(bt)->hi_as_long();
> 1142:   Node *casted_length = ConstraintCastNode::make_cast_for_basic_type(

Suggestion:

  Node* casted_length = ConstraintCastNode::make_cast_for_basic_type(

src/hotspot/share/opto/library_call.cpp line 1172:

> 1170: 
> 1171:   // index is now known to be >= 0 and < length, cast it
> 1172:   Node *result = ConstraintCastNode::make_cast_for_basic_type(

Suggestion:

  Node* result = ConstraintCastNode::make_cast_for_basic_type(

src/hotspot/share/opto/loopTransform.cpp line 3423:

> 3421: 
> 3422:   // We need to pin the exact limit to prevent it from floating above the zero trip guard.
> 3423:   Node *cast_ii = ConstraintCastNode::make_cast_for_basic_type(

Suggestion:

  Node* cast_ii = ConstraintCastNode::make_cast_for_basic_type(

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17162#pullrequestreview-1794200611
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1434877087
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1434881955
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1434880223
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1434880324
PR Review Comment: https://git.openjdk.org/jdk/pull/17162#discussion_r1434880759


More information about the hotspot-compiler-dev mailing list