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