RFR: 8333684: C2 SuperWord: multiple smaller refactorings in preparation for JDK-8332163
Emanuel Peter
epeter at openjdk.org
Sat Jun 8 16:06:42 UTC 2024
On Thu, 6 Jun 2024 07:31:53 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> In preparation for https://github.com/openjdk/jdk/pull/19261, I made some smaller refactorings / moving code around:
>
> - `SuperWord::same_input` -> `PackSet::isa_unique_input_or_null`
> - Rename print-method tags: `SUPERWORD1_BEFORE_SCHEDULE` -> `AUTO_VECTORIZATION1_BEFORE_APPLY` etc.
> - Refactored `SuperWord::schedule / output` -> `SuperWord::schedule_and_apply`:
> - Reorganize so that we can separate out all methods that change the C2 graph into `SuperWord::apply`.
> - Move all `phase()->C->print_method` to `SuperWord::apply`.
> - Rename `SuperWord::schedule_reorder_memops` -> `SuperWord::apply_memops_reordering_with_schedule`.
> - Rename `SuperWord::output` -> `SuperWord::apply_vectorization`.
> - Move `SuperWord::vectors_should_be_aligned` -> `VLoop::vectors_should_be_aligned`.
> - Move `SuperWord::requires_long_to_int_conversion` -> `VectorNode::is_scalar_op_that_returns_int_but_vector_op_returns_long`, and move comments.
> - Move `VectorNode::can_transform_shift_op` -> `VectorNode::can_use_RShiftI_instead_of_URShiftI`, and move comments.
> - Extract out `PackSet::get_bool_test` from `SuperWord::output / apply_vectorization`.
> - Extract opcode check to `VectorNode::is_scalar_unary_op_with_equal_input_and_output_types`.
src/hotspot/share/opto/phasetype.hpp line 74:
> 72: flags(AUTO_VECTORIZATION2_AFTER_REORDER, "AutoVectorization 2, After Apply Memop Reordering") \
> 73: flags(AUTO_VECTORIZATION3_AFTER_ADJUST_LIMIT, "AutoVectorization 3, After Adjusting Pre-Loop Limit") \
> 74: flags(AUTO_VECTORIZATION4_AFTER_APPLY, "AutoVectorization 4, After Apply") \
Note: Renamed and added a 4th tag.
src/hotspot/share/opto/superword.cpp line 488:
> 486:
> 487: DEBUG_ONLY(verify_packs();)
> 488: DEBUG_ONLY(verify_no_extract());
Note: moved from `SuperWord::output`
src/hotspot/share/opto/superword.cpp line 490:
> 488: DEBUG_ONLY(verify_no_extract());
> 489:
> 490: return schedule_and_apply();
Note: refactored `schedule` and `output` -> `schedule_and_apply`.
src/hotspot/share/opto/superword.cpp line 1645:
> 1643: return true;
> 1644: default:
> 1645: return false;
Note: moved to `VectorNode::is_scalar_op_that_returns_int_but_vector_op_returns_long`
src/hotspot/share/opto/superword.cpp line 1659:
> 1657: Node* pi_def = pi->in(idx);
> 1658: if (p0_def != pi_def) {
> 1659: return false;
Note: refactored `SuperWord::same_inputs` -> `PackSet::isa_unique_input_or_null`.
src/hotspot/share/opto/superword.cpp line 1779:
> 1777: retValue = UseVectorCmov;
> 1778: } else if (VectorNode::is_scalar_op_that_returns_int_but_vector_op_returns_long(opc)) {
> 1779: // Requires extra vector long -> int conversion.
Note: moved comments to method definition. Chose better name.
src/hotspot/share/opto/superword.cpp line 1783:
> 1781: VectorCastNode::implemented(Op_ConvL2I, size, T_LONG, T_INT);
> 1782: } else {
> 1783: if (VectorNode::can_use_RShiftI_instead_of_URShiftI(p0, velt_basic_type(p0))) {
Note: moved comments to method definition. Chose better name.
src/hotspot/share/opto/superword.cpp line 1885:
> 1883: // <==> VectorBlend( VectorMaskCmp(GT_O, in1_cmp, in2_cmp), in2_blend, in1_blend)
> 1884: mask = bol0->_test.negate();
> 1885: is_negated = true;
Note: extracted from `CMove` code in `SuperWord::output`
src/hotspot/share/opto/superword.cpp line 2090:
> 2088: memops_schedule.dump();
> 2089: }
> 2090: #endif
Note: debug printing moved to `SuperWord::apply_memops_reordering_with_schedule`
src/hotspot/share/opto/superword.cpp line 2208:
> 2206: adjust_pre_loop_limit_to_align_main_loop_vectors();
> 2207:
> 2208: DEBUG_ONLY(verify_no_extract());
Note: moved these out earlier.
src/hotspot/share/opto/superword.cpp line 2316:
> 2314: // (4) Apply the vectorization, including re-ordering the memops.
> 2315: return apply(memops_schedule);
> 2316: }
Note: instead of just `schedule_reorder_memops`, we refactor and call `apply`, which does all the C2 graph hacking, including the `schedule_reorder_memops` code.
src/hotspot/share/opto/superword.cpp line 2332:
> 2330:
> 2331: return is_success;
> 2332: }
Note: refactored out all the "apply" code into a nice list, and do the `print_method` code right there too.
src/hotspot/share/opto/superword.cpp line 2434:
> 2432: assert(cl->is_main_loop(), "SLP should only work on main loops");
> 2433: Compile* C = phase()->C;
> 2434: assert(!_packset.is_empty(), "vectorization requires non-empty packset");
Note: runtime check already happens earlier/further up, in `SuperWord::schedule_and_apply`.
src/hotspot/share/opto/superword.cpp line 2529:
> 2527:
> 2528: phase()->C->print_method(PHASE_SUPERWORD3_AFTER_OUTPUT, 4, cl);
> 2529:
Note: moved to `SuperWord::apply`
src/hotspot/share/opto/superword.cpp line 2548:
> 2546: if (bool_test._is_negated) {
> 2547: // We can cancle out the negation by swapping the blend inputs.
> 2548: swap(blend_in1, blend_in2);
Note: refactored out to `_packset.get_bool_test`. This will make future refactorings around `SuperWord::apply_vectorization` easier.
src/hotspot/share/opto/superword.cpp line 2597:
> 2595: }
> 2596: } else {
> 2597: if (VectorNode::can_use_RShiftI_instead_of_URShiftI(n, velt_basic_type(n))) {
Note: moved comments to method definition, and gave it a better name.
src/hotspot/share/opto/superword.cpp line 2603:
> 2601: vlen_in_bytes = vn->as_Vector()->length_in_bytes();
> 2602: }
> 2603: } else if (VectorNode::is_scalar_unary_op_with_equal_input_and_output_types(opc)) {
Note: captured opcode check to a dedicated method.
src/hotspot/share/opto/superword.cpp line 2608:
> 2606: vn = VectorNode::make(opc, in, nullptr, vlen, velt_basic_type(n));
> 2607: vlen_in_bytes = vn->as_Vector()->length_in_bytes();
> 2608: } else if (VectorNode::is_scalar_op_that_returns_int_but_vector_op_returns_long(opc)) {
Note: moved comment to definition, gave it better name.
src/hotspot/share/opto/vectorization.hpp line 1326:
> 1324: VTransformBoolTest(const BoolTest::mask mask, bool is_negated) :
> 1325: _mask(mask), _is_negated(is_negated) {}
> 1326: };
Note: used by `PackSet::get_bool_test`. I gave it a `VTransform` name in anticipation of the `VTransform` refactoring in https://github.com/openjdk/jdk/pull/19261
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629251270
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629252612
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629252165
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629255418
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629256254
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629253375
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629253937
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629256842
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629258095
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629263409
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629259838
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629261329
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629262955
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629267393
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629265060
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629265742
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629266323
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629266859
PR Review Comment: https://git.openjdk.org/jdk/pull/19573#discussion_r1629268921
More information about the hotspot-compiler-dev
mailing list