RFR: 8325064: C2 SuperWord: refactor construct_bb

[Emanuel Peter]

On Wed, 31 Jan 2024 16:53:59 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> Subtask of https://github.com/openjdk/jdk/pull/16620
> 
> The goal is to further disentangle different "components" in Superword.
> 
> In this refactoring, I disentangle the `bb`, `reduction` and `memory_slice` "components" which were all intertwined in `construct_bb`.
> 
> 1. Move memory slice code -> `analyze_memory_slices`.
> 2. Remove reduction checking code -> simply use the `is_marked_reduction_loop` condition outside.
> 3. `_data_entry`: was used for non-CFG nodes in the loop that have no input node that is also inside the loop. But that actually never happens! I removed that array, and replaced the code with verification.

src/hotspot/share/opto/superword.cpp line 535:

> 533: #endif
> 534:     return false;
> 535:   }

Note: this condition used to be at the end of `SuperWord::construct_bb`. Now it makes more sense to do this outside.

src/hotspot/share/opto/superword.cpp line 549:

> 547: 
> 548:   // Ensure extra info is allocated.
> 549:   initialize_node_info();

Note: used to be `initialize_bb` inside `construct_bb`. Corrected the name and moved it out.

src/hotspot/share/opto/superword.cpp line 949:

> 947:   }
> 948: }
> 949: #endif

Note: both of these methods are refactored out of `construct_bb`

src/hotspot/share/opto/superword.cpp line 2988:

> 2986:           assert(n != entry, "can't be entry");
> 2987:           _data_entry.push(n);
> 2988:         }

Note: `found` is always true, it turns out. I added an assert. And this means I can also remove `_data_entry`, as we now never push anything to it.

src/hotspot/share/opto/superword.cpp line 2994:

> 2992: 
> 2993:   // Find memory slices (head and tail)
> 2994:   for (DUIterator_Fast imax, i = lp()->fast_outs(imax); i < imax; i++) {

Note: moved the memory slice code to `analyze_memory_slices`

src/hotspot/share/opto/superword.cpp line 3040:

> 3038:             // Don't go around backedge
> 3039:             (!use->is_Phi() || n == entry)) {
> 3040:           if (is_marked_reduction(use)) {

Note: I would like to remove this reduction code from the basic block code. It separates the different "components".

Instead, we can just check `is_marked_reduction_loop` outside, which checks if we have any marked reduction.
The only difference is that we don't do the implemented check. But most platforms implement the reductions, and we check that again later in `SuperWord::implemented` anyway.

src/hotspot/share/opto/superword.cpp line 3094:

> 3092: #endif
> 3093:   assert(rpo_idx == -1 && bb_ct == _block.length(), "all block members found");
> 3094:   return (_mem_slice_head.length() > 0) || (reduction_uses > 0) || (_data_entry.length() > 0);

Note: Moved this condition out:

  if (!is_marked_reduction_loop() &&
      _mem_slice_head.is_empty()) {

src/hotspot/share/opto/superword.cpp line 3123:

> 3121:   }
> 3122: }
> 3123: 

Note: not used anymore, already prevous to this change

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473158354
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473159421
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473160065
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473163417
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473162067
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473167670
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473168907
PR Review Comment: https://git.openjdk.org/jdk/pull/17657#discussion_r1473169382