RFR: 8346774: Use Predicate classes instead of Node classes

[Christian Hagedorn]

On Wed, 22 Jan 2025 13:10:37 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> This small cleanup PR replaces a lot of usages of `Node` pointers, to pass around either the head (i.e. `IfNode`) or the tail (i.e. a success projection) of predicates, with actual `Predicate` classes. This simplifies the usages, readability and the logical flow, and enables more simplifications in the future, especially once we replace Template Assertion Predicates with a dedicated node.
> 
> I've also included some minor refactorings like adding `const` or fixing typos.
> 
> There are no semantic changes involved. The return value optimization should take care to avoid a lot of copies when returning new objects from methods. 
> 
> Thanks,
> Christian

src/hotspot/share/opto/predicates.cpp line 193:

> 191:   DEBUG_ONLY(verify();)
> 192:   OpaqueLoopInitNode* new_opaque_init = new OpaqueLoopInitNode(phase->C, new_opaque_input);
> 193:   phase->register_new_node(new_opaque_init, new_control);

Moved here from the caller of `clone_and_replace_init()`. He now provides the input to the opaque node instead of creating the opaque node and passing the created node to this method.

src/hotspot/share/opto/predicates.cpp line 214:

> 212: 
> 213: // Create a new Initialized Assertion Predicate from this template at the template success projection.
> 214: InitializedAssertionPredicate TemplateAssertionPredicate::initialize(PhaseIdealLoop* phase) const {

Removed `new_control`. All callers created the Initialized Assertion Predicate below the template. Eventually, this will always be the case since we want to keep Template Assertion Predicates around in the future.

src/hotspot/share/opto/predicates.cpp line 807:

> 805: //
> 806: InitializedAssertionPredicate InitializedAssertionPredicateCreator::create_from_template(
> 807:     IfNode* template_assertion_predicate, Node* new_control, Node* new_init, Node* new_stride) const {

Also made `const` and split long line.

src/hotspot/share/opto/predicates.cpp line 819:

> 817: // projection by cloning it but omitting the OpaqueLoop*Notes (i.e. taking their inputs instead).
> 818: InitializedAssertionPredicate InitializedAssertionPredicateCreator::create_from_template_and_insert_below(
> 819:     const TemplateAssertionPredicate& template_assertion_predicate) const {

Same as above: Removed `new_control`.
Also made `const`.

src/hotspot/share/opto/predicates.cpp line 827:

> 825:   IfNode* template_assertion_predicate_if = template_assertion_predicate.head();
> 826:   AssertionPredicateType assertion_predicate_type = template_assertion_predicate_if->assertion_predicate_type();
> 827:   int if_opcode = template_assertion_predicate_if->Opcode();

Need to fetch the information from the Template Assertion Predicate now instead of directly from the `If`.

src/hotspot/share/opto/predicates.cpp line 963:

> 961:   OpaqueLoopInitNode* opaque_init = new OpaqueLoopInitNode(_phase->C, _init);
> 962:   _phase->register_new_node(opaque_init, _old_target_loop_entry);
> 963:   return template_assertion_predicate.clone_and_replace_init(_old_target_loop_entry, opaque_init, _phase);

See earlier comment about moving opaque creation from caller -> callee.

src/hotspot/share/opto/predicates.cpp line 1029:

> 1027:   _current_predicate_chain_head = predicate.head();
> 1028:   assert(predicate.head()->_idx >= _node_index_before_cloning, "must be a newly cloned predicate");
> 1029:   assert(predicate.tail()->_idx >= _node_index_before_cloning, "must be a newly cloned predicate");

Can now verify both the head and tail.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925298955
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925300491
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925301310
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925303798
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925304681
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925306010
PR Review Comment: https://git.openjdk.org/jdk/pull/23234#discussion_r1925307593