RFR: 8342047: Create Template Assertion Predicates with Halt nodes only instead of uncommon traps
Christian Hagedorn
chagedorn at openjdk.org
Wed Nov 13 12:07:27 UTC 2024
On Tue, 12 Nov 2024 15:06:52 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
> This patch replaces the creation of Template Assertion Predicates with uncommon traps with Halt nodes.
>
> ### Goal of Assertion Predicates
> #### Initialized Assertion Predicates
> These predicates ensure that control is properly folded when data is dying. They are **always true by design** and thus can never fail at runtime. We therefore put a halt node on the failing path.
>
> #### Template Assertion Predicates
> Only serve as templates to create Initialized Assertion Predicates from. They are never executed and are always removed after loop opts are over. Conceptionally, it does not matter whether the failing path uses an UCT or a halt node (or something else completely - I plan to have a separate "no-op" `TemplateAssertionPredicateNode` at some point which only falls through to the next node and does not have a failing path at all).
>
> ### Why Did we Use UCTs for Template Assertion Predicates?
> When the concept of Assertion Predicates was first introduced, it only covered a few edge cases. It was quite straight forward to reuse existing Loop Predication code which creates new predicates from a Parse Predicate by copying it and merging the UCTs on the failing paths with a region node. This was done with `PhaseIdealLoop::create_new_if_for_predicate()`.
>
> ### Why Do we Need to Use Halt Nodes for Template Assertion Predicates?
> #### Missing UCTs for Predicates above Loops
> Over time, we found more cases where we need to create Initialized Assertion Predicates from templates - including locations where we do not have Parse Predicates (and thus no safepoints). For example, when peeling one iteration off a loop with Parse Predicates, they will be kept at the peeled iteration and the remaining loop does not have any Parse Predicates anymore.
>
> #### Missing UCTs to Create Template Assertion Predicates
> Whenever we split a loop with Template Assertion Predicates, we also need to ensure that they are copied to all split loop versions. Since they rely on using UCTs, we also need to make sure that an UCT/safepoint is available to be used. However, this is not always the case (for example, after peeling an iteration off as described in the last section). As a result, we cannot easily establish new Template Assertion Predicates anywhere. One could think about faking an UCT or doing other special logic. But this seems rather fragile and could introduce quite some complexity - especially since we conceptionally don't even need to use UCTs at all for Template Assertion Predicates.
>
> There ...
Just noticed that I forgot to submit my accompanying comment.
src/hotspot/share/opto/loopPredicate.cpp line 290:
> 288: // cloned predicates.
> 289: void PhaseIdealLoop::clone_assertion_predicates_to_unswitched_loop(IdealLoopTree* loop, const Node_List& old_new,
> 290: Deoptimization::DeoptReason reason,
unused
src/hotspot/share/opto/loopPredicate.cpp line 296:
> 294: // original predicate order.
> 295: Unique_Node_List list;
> 296: get_template_assertion_predicates(old_parse_predicate_proj, list);
More precise since we only care about Template Assertion Predicates.
src/hotspot/share/opto/loopPredicate.cpp line 304:
> 302: Node_List to_process;
> 303: // Process in reverse order such that 'create_new_if_for_predicate' can be used in
> 304: // 'clone_assertion_predicate_for_unswitched_loops' and the original order is maintained.
Covered by updated comment on L293.
src/hotspot/share/opto/loopPredicate.cpp line 313:
> 311: for (DUIterator j = template_assertion_predicate_success_proj->outs();
> 312: template_assertion_predicate_success_proj->has_out(j);
> 313: j++) {
renaming `predicate` -> `template_assertion_predicate_success_proj`
src/hotspot/share/opto/loopPredicate.cpp line 315:
> 313: assert(assertion_predicate_has_loop_opaque_node(fast_proj->in(0)->as_If()), "must find Assertion Predicate for fast loop");
> 314: IfProjNode* slow_proj = clone_assertion_predicate_for_unswitched_loops(iff, predicate_proj, reason, slow_loop_parse_predicate_proj);
> 315: assert(assertion_predicate_has_loop_opaque_node(slow_proj->in(0)->as_If()), "must find Assertion Predicate for slow loop");
`assert` moved to `clone_assertion_predicate_for_unswitched_loops()`.
src/hotspot/share/opto/loopPredicate.cpp line 350:
> 348: ParsePredicateNode* unswitched_loop_parse_predicate) {
> 349: TemplateAssertionPredicate template_assertion_predicate(template_assertion_predicate_success_proj);
> 350: IfTrueNode* template_success_proj = template_assertion_predicate.clone(unswitched_loop_parse_predicate->in(0), this);
Introduced new `clone()` method to do the cloning without an UCT and thus we no longer use `create_new_if_for_predicate()`.
src/hotspot/share/opto/loopPredicate.cpp line 362:
> 360: void PhaseIdealLoop::clone_parse_and_assertion_predicates_to_unswitched_loop(IdealLoopTree* loop, Node_List& old_new,
> 361: IfProjNode*& true_path_loop_entry,
> 362: IfProjNode*& false_path_loop_entry) {
Should have renamed these earlier: We no longer call the unswitched loop versions fast and slow but rather true path and false path.
src/hotspot/share/opto/loopPredicate.cpp line 1292:
> 1290:
> 1291: TemplateAssertionPredicateCreator template_assertion_predicate_creator(loop_head, scale, offset, range, this);
> 1292: IfTrueNode* template_success_proj = template_assertion_predicate_creator.create(new_control);
Reuse existing `TemplateAssertionPredicateCreator.create()` (which was named `create_with_halt()` before) to create a new Template Assertion Predicate with a halt node.
src/hotspot/share/opto/loopnode.cpp line 2837:
> 2835: }
> 2836: if (is_main_loop() || is_post_loop()) {
> 2837: AssertionPredicates assertion_predicates(ctrl);
Dropped the `WithHalt` postfix since we only have Assertion Predicate with Halt nodes now.
src/hotspot/share/opto/predicates.cpp line 85:
> 83: }
> 84:
> 85: Deoptimization::DeoptReason RuntimePredicate::uncommon_trap_reason(IfProjNode* if_proj) {
We no longer have Template Assertion Predicates with UCTs. Thus, we could move this check to `RuntimePredicate` which use UCTs.
src/hotspot/share/opto/predicates.cpp line 155:
> 153:
> 154: // Clone this Template Assertion Predicate and replace the OpaqueLoopInitNode with the provided 'new_opaque_init' node.
> 155: IfTrueNode* TemplateAssertionPredicate::clone(Node* new_control, PhaseIdealLoop* phase) const {
New `clone()` method which creates a Template Assertion Predicate with a halt node by reusing the existing `AssertionPredicateIfCreator` class.
src/hotspot/share/opto/predicates.cpp line 622:
> 620: // Creates an init and last value Template Assertion Predicate connected together from a Parse Predicate with an UCT on
> 621: // the failing path. Returns the success projection of the last value Template Assertion Predicate.
> 622: IfTrueNode* TemplateAssertionPredicateCreator::create_with_uncommon_trap(
No longer required.
src/hotspot/share/opto/predicates.cpp line 653:
> 651: }
> 652:
> 653: IfTrueNode* TemplateAssertionPredicateCreator::create_if_node_with_uncommon_trap(
No longer required.
src/hotspot/share/opto/predicates.hpp line 300:
> 298: static bool is_predicate(const Node* node, Deoptimization::DeoptReason deopt_reason);
> 299: static bool has_valid_uncommon_trap(const Node* success_proj);
> 300: };
No longer required. Most of these method have been moved to the `RuntimePredicate` class.
src/hotspot/share/opto/predicates.hpp line 459:
> 457: public:
> 458: OpaqueTemplateAssertionPredicateNode* clone(Node* new_control, PhaseIdealLoop* phase);
> 459: OpaqueTemplateAssertionPredicateNode* clone_and_replace_init(Node* new_control, Node* new_init,
Renamed `new_ctrl` -> `new_control` and swapped order of parameters.
src/hotspot/share/opto/predicates.hpp line 590:
> 588: ParsePredicateSuccessProj* parse_predicate_success_proj,
> 589: Deoptimization::DeoptReason deopt_reason, int if_opcode,
> 590: bool does_overflow, AssertionPredicateType assertion_predicate_type);
No longer required.
src/hotspot/share/opto/predicates.hpp line 591:
> 589: NONCOPYABLE(TemplateAssertionPredicateCreator);
> 590:
> 591: IfTrueNode* create(Node* new_control);
Renamed to `create()`.
src/hotspot/share/opto/predicates.hpp line 593:
> 591: IfTrueNode* create_if_node_with_halt(Node* new_control,
> 592: OpaqueTemplateAssertionPredicateNode* template_assertion_predicate_expression,
> 593: bool does_overflow, AssertionPredicateType assertion_predicate_type);
Renamed to `create_if_node()`.
src/hotspot/share/opto/predicates.hpp line 606:
> 604:
> 605: IfTrueNode* create_with_uncommon_trap(Node* new_control, ParsePredicateSuccessProj* parse_predicate_success_proj,
> 606: Deoptimization::DeoptReason deopt_reason, int if_opcode);
No longer required.
-------------
PR Review: https://git.openjdk.org/jdk/pull/22040#pullrequestreview-2429875583
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838268869
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838269937
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838270663
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838272261
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838271271
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838274015
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838275182
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838278848
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838279751
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838281378
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838306376
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838303944
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838303822
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838303121
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838300383
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838301020
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838302058
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838301285
PR Review Comment: https://git.openjdk.org/jdk/pull/22040#discussion_r1838302243
More information about the hotspot-compiler-dev
mailing list