https://gcc.gnu.org/g:30c751b18eaf6e72c6d1ecf46aa5e28122069a41
commit r17-5402-g30c751b18eaf6e72c6d1ecf46aa5e28122069a41 Author: Jakub Jelinek <[email protected]> Date: Tue Oct 20 12:09:47 2026 +0200 cse: Fix up cse_vec_duplicate_match optimization [PR127983] In this testcase we have: (insn 12 11 13 4 (set (reg:V4SI 108) (vec_duplicate:V4SI (reg/v:SI 104 [ s ]))) "pr127983.c":13:9 9532 {*vec_dupv4si} (nil)) ... (insn 15 14 16 4 (parallel [ (set (reg/v:SI 104 [ s ]) (plus:SI (reg/v:SI 104 [ s ]) (const_int 1 [0x1]))) (clobber (reg:CC 17 flags)) ]) "pr127983.c":14:8 288 {*addsi_1} (nil)) (insn 16 15 17 4 (set (reg:V8SI 109) (vec_duplicate:V8SI (reg/v:SI 104 [ s ]))) "pr127983.c":15:9 9543 {*vec_dupv8si} (nil)) and cse1 turns that since r17-2354 incorrectly into: (insn 16 11 12 4 (set (reg:V8SI 109) (vec_duplicate:V8SI (reg/v:SI 104 [ s ]))) "pr127983.c":15:9 9543 {*vec_dupv8si} (nil)) (insn 12 16 13 4 (set (reg:V4SI 108) (subreg:V4SI (reg:V8SI 109) 0)) "pr127983.c":13:9 2557 {movv4si_internal} (nil)) ... (insn 15 14 16 4 (parallel [ (set (reg/v:SI 104 [ s ]) (plus:SI (reg/v:SI 104 [ s ]) (const_int 1 [0x1]))) (clobber (reg:CC 17 flags)) ]) "pr127983.c":14:8 288 {*addsi_1} (nil)) (insn 17 15 18 4 (set (reg:V8SI 101 [ _2 ]) (reg:V8SI 109)) "pr127983.c":15:9 2556 {movv8si_internal} (expr_list:REG_DEAD (reg:V8SI 109) (nil))) That is wrong because pseudo 104 has been modified in between the two vec_duplicate insns. The optimization already works solely within each bb rather than ebb (it truncates vec_duplicate_cache at the start of each bb), so the current /* Safety check that WIDE_REG is not otherwise used or redefined before its original defining insn. */ if (reg_used_between_p (wide_reg, match.first_insn, match.widest_insn) || reg_set_between_p (wide_reg, match.first_insn, match.widest_insn)) continue; can work properly (on ebbs it would need to separately check in each bb along the ebb path I think). And to fix this bug we just need to also check whether scalar (a REG or SUBREG of a REG) hasn't changed in between. So that it isn't O(n^2) in case of many VEC_DUPLICATEs, this patch remembers the last corresponding VEC_DUPLICATE in the bb and only checks for reg_set_between_p since then. If the new member is NULL, it means the scalar has been or could have been modified and so we don't try to reuse/extend it anymore. Also, the if and else if conditions in the loop had some common subconditions, I've moved them to an outer if to avoid duplication, and finally the r17-2354 added code in cse_main had wrong indentation level, so I've reindented it. 2026-10-20 Jakub Jelinek <[email protected]> PR rtl-optimization/127983 * cse.cc (struct cse_vec_duplicate_match): Add last_checked_insn, initialize it in both constructors. Formatting fix. (cse_prescan_cache_vec_dup): Check common subconditions between if and else if once. Also give up if entry.last_checked_insn is NULL. If scalar is set in between last_checked_insn and insn, clear last_checked_insn and don't record a dup, otherwise update last_checked_insn. Adjust new_entry construction. (cse_main): Reindent the vec_duplicate_match code. * gcc.target/i386/avx2-pr127983.c: New test. Reviewed-by: Richard Biener <[email protected]> Diff: --- gcc/cse.cc | 258 ++++++++++++++------------ gcc/testsuite/gcc.target/i386/avx2-pr127983.c | 25 +++ 2 files changed, 165 insertions(+), 118 deletions(-) diff --git a/gcc/cse.cc b/gcc/cse.cc index 697d03f32511..d3ada53db3e2 100644 --- a/gcc/cse.cc +++ b/gcc/cse.cc @@ -493,21 +493,25 @@ struct cse_vec_duplicate_match rtx scalar; rtx_insn *first_insn; rtx_insn *widest_insn; + rtx_insn *last_checked_insn; auto_vec<rtx_insn *> related_dups; cse_vec_duplicate_match (basic_block bb_, machine_mode widest_mode_, rtx scalar_, rtx_insn *first_insn_, - rtx_insn *widest_insn_) + rtx_insn *widest_insn_, + rtx_insn *last_checked_insn_) : bb (bb_), widest_mode (widest_mode_), scalar (scalar_), - first_insn (first_insn_), widest_insn (widest_insn_) + first_insn (first_insn_), widest_insn (widest_insn_), + last_checked_insn (last_checked_insn_) {} cse_vec_duplicate_match (const cse_vec_duplicate_match &other) : bb (other.bb), widest_mode (other.widest_mode), scalar (other.scalar), - first_insn (other.first_insn), widest_insn (other.widest_insn) + first_insn (other.first_insn), widest_insn (other.widest_insn), + last_checked_insn (other.last_checked_insn) { - related_dups.reserve(other.related_dups.length ()); - related_dups.splice(other.related_dups); + related_dups.reserve (other.related_dups.length ()); + related_dups.splice (other.related_dups); } }; @@ -6591,30 +6595,43 @@ cse_prescan_cache_vec_dup (struct cse_basic_block_data *data, basic_block bb, /* Check for matching cached vec_duplicates and save the match if found. */ for (auto &entry : data->vec_duplicate_cache) - { - /* Create a match with existing cache entry if both scalar register - pseudos are matching and the new vec_duplicate mode is wider. */ - if (REG_P (dest) && rtx_equal_p (entry.scalar, scalar) - && known_gt (GET_MODE_SIZE (mode), GET_MODE_SIZE (entry.widest_mode)) - && GET_MODE_INNER (mode) == GET_MODE_INNER (entry.widest_mode)) - { - entry.widest_mode = mode; - entry.widest_insn = insn; - entry.related_dups.safe_push (insn); - return; - } - else if (GET_MODE_INNER (mode) == GET_MODE_INNER (entry.widest_mode) - && rtx_equal_p (entry.scalar, scalar) - && known_le (GET_MODE_SIZE (mode), - GET_MODE_SIZE (entry.widest_mode))) - { - entry.related_dups.safe_push (insn); - return; - } - } + /* Create a match with existing cache entry if both scalar register + pseudos are matching and the new vec_duplicate mode is wider. */ + if (GET_MODE_INNER (mode) == GET_MODE_INNER (entry.widest_mode) + && rtx_equal_p (entry.scalar, scalar) + && entry.last_checked_insn) + { + if (REG_P (dest) + && known_gt (GET_MODE_SIZE (mode), + GET_MODE_SIZE (entry.widest_mode))) + { + if (reg_set_between_p (scalar, entry.last_checked_insn, insn)) + { + entry.last_checked_insn = NULL; + continue; + } + entry.widest_mode = mode; + entry.widest_insn = insn; + entry.last_checked_insn = insn; + entry.related_dups.safe_push (insn); + return; + } + else if (known_le (GET_MODE_SIZE (mode), + GET_MODE_SIZE (entry.widest_mode))) + { + if (reg_set_between_p (scalar, entry.last_checked_insn, insn)) + { + entry.last_checked_insn = NULL; + continue; + } + entry.last_checked_insn = insn; + entry.related_dups.safe_push (insn); + return; + } + } /* Cache vec_duplicate as a new entry if no match was found. */ - cse_vec_duplicate_match new_entry (bb, mode, scalar, insn, NULL); + cse_vec_duplicate_match new_entry (bb, mode, scalar, insn, NULL, insn); new_entry.related_dups.safe_push (insn); data->vec_duplicate_cache.safe_push (new_entry); } @@ -6925,112 +6942,117 @@ cse_main (rtx_insn *f ATTRIBUTE_UNUSED, int nregs) if (ebb_data.nsets == 0) continue; - /* If prescan discovers any vec_duplicate pairs where a wider duplicate - appears after a narrow duplicate of the same scalar, move the - widest duplicate before the first vec_duplicate of this scalar and - rewrite all related duplicates to reuse it. */ - for (auto &match : ebb_data.vec_duplicate_matches) - { - rtx wide_set = single_set (match.widest_insn); - if (!wide_set) - continue; - rtx wide_reg = SET_DEST (wide_set); - rtx_insn *insert_after = PREV_INSN (match.first_insn); - if (!REG_P (wide_reg) || !insert_after) - continue; + /* If prescan discovers any vec_duplicate pairs where a wider + duplicate appears after a narrow duplicate of the same scalar, + move the widest duplicate before the first vec_duplicate of this + scalar and rewrite all related duplicates to reuse it. */ + for (auto &match : ebb_data.vec_duplicate_matches) + { + rtx wide_set = single_set (match.widest_insn); + if (!wide_set) + continue; + rtx wide_reg = SET_DEST (wide_set); + rtx_insn *insert_after = PREV_INSN (match.first_insn); + if (!REG_P (wide_reg) || !insert_after) + continue; - /* Safety check that WIDE_REG is not otherwise used or redefined - before its original defining insn. */ - if (reg_used_between_p (wide_reg, match.first_insn, - match.widest_insn) - || reg_set_between_p (wide_reg, match.first_insn, - match.widest_insn)) - continue; + /* Safety check that WIDE_REG is not otherwise used or redefined + before its original defining insn. */ + if (reg_used_between_p (wide_reg, match.first_insn, + match.widest_insn) + || reg_set_between_p (wide_reg, match.first_insn, + match.widest_insn)) + continue; - /* Rewrite all related duplicates as a group so we either keep the - whole transformation or none of it. */ - int prev_changes = num_changes_pending (); - bool abort_changes = false; + /* Rewrite all related duplicates as a group so we either keep + the whole transformation or none of it. */ + int prev_changes = num_changes_pending (); + bool abort_changes = false; - for (auto &dup_insn : match.related_dups) - { - rtx dup_set; - rtx dup_src; - machine_mode dup_mode; + for (auto &dup_insn : match.related_dups) + { + rtx dup_set; + rtx dup_src; + machine_mode dup_mode; - if (dup_insn == match.widest_insn) - continue; + if (dup_insn == match.widest_insn) + continue; - /* The earlier safety check already covered the range from - FIRST_INSN up to WIDEST_INSN. Only check the remaining - suffix for duplicates that come later in the block. This - also covers call-clobbered hard registers via - reg_set_between_p. */ - bool reg_set = false; - bool reg_set_with_widest_after = false; - for (rtx_insn *scan = NEXT_INSN (match.widest_insn); - scan != NEXT_INSN (BB_END (match.bb)) && scan != NULL_RTX; - scan = NEXT_INSN (scan)) - { - if (scan == dup_insn) - { - reg_set_with_widest_after = reg_set; - break; - } - if (!reg_set && INSN_P (scan) && reg_set_p (wide_reg, scan)) - reg_set = true; - } + /* The earlier safety check already covered the range from + FIRST_INSN up to WIDEST_INSN. Only check the remaining + suffix for duplicates that come later in the block. This + also covers call-clobbered hard registers via + reg_set_between_p. */ + bool reg_set = false; + bool reg_set_with_widest_after = false; + for (rtx_insn *scan = NEXT_INSN (match.widest_insn); + scan != NEXT_INSN (BB_END (match.bb)) + && scan != NULL_RTX; + scan = NEXT_INSN (scan)) + { + if (scan == dup_insn) + { + reg_set_with_widest_after = reg_set; + break; + } + if (!reg_set + && INSN_P (scan) + && reg_set_p (wide_reg, scan)) + reg_set = true; + } - if (reg_set_with_widest_after) - continue; + if (reg_set_with_widest_after) + continue; - dup_set = single_set (dup_insn); - if (!dup_set) - { - abort_changes = true; - break; - } + dup_set = single_set (dup_insn); + if (!dup_set) + { + abort_changes = true; + break; + } - dup_mode = GET_MODE (SET_DEST (dup_set)); - if (dup_mode == GET_MODE (wide_reg)) - dup_src = wide_reg; - else - { - dup_src = gen_lowpart (dup_mode, wide_reg); - if (!dup_src) - { - abort_changes = true; - break; - } - } + dup_mode = GET_MODE (SET_DEST (dup_set)); + if (dup_mode == GET_MODE (wide_reg)) + dup_src = wide_reg; + else + { + dup_src = gen_lowpart (dup_mode, wide_reg); + if (!dup_src) + { + abort_changes = true; + break; + } + } - if (rtx_equal_p (dup_src, SET_DEST (dup_set))) - continue; + if (rtx_equal_p (dup_src, SET_DEST (dup_set))) + continue; - if (!validate_change (dup_insn, &SET_SRC (dup_set), dup_src, 1)) - { - abort_changes = true; - break; - } - } + if (!validate_change (dup_insn, &SET_SRC (dup_set), + dup_src, 1)) + { + abort_changes = true; + break; + } + } - if (abort_changes) - { - cancel_changes (prev_changes); - continue; - } + if (abort_changes) + { + cancel_changes (prev_changes); + continue; + } - if (num_changes_pending () == prev_changes) - continue; + if (num_changes_pending () == prev_changes) + continue; - if (!apply_change_group ()) - continue; + if (!apply_change_group ()) + continue; - reorder_insns (match.widest_insn, match.widest_insn, insert_after); + reorder_insns (match.widest_insn, match.widest_insn, + insert_after); - if (match.related_dups.contains (match.widest_insn)) - df_insn_rescan (match.widest_insn); - } + if (match.related_dups.contains (match.widest_insn)) + df_insn_rescan (match.widest_insn); + } /* Get a reasonable estimate for the maximum number of qty's needed for this path. For this, we take the number of sets diff --git a/gcc/testsuite/gcc.target/i386/avx2-pr127983.c b/gcc/testsuite/gcc.target/i386/avx2-pr127983.c new file mode 100644 index 000000000000..82f5963acfe3 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx2-pr127983.c @@ -0,0 +1,25 @@ +/* PR rtl-optimization/127983 */ +/* { dg-do run { target avx2 } } */ +/* { dg-options "-O2 -mavx2" } */ + +#include "avx2-check.h" + +typedef int v4si __attribute__((vector_size (16))); +typedef int v8si __attribute__((vector_size (32))); +volatile v4si x; +volatile v8si y; + +static void +avx2_test (void) +{ + volatile int z = 1; + int s = z; + for (int i = 0; i < z; i++) + { + x = (v4si) { s, s, s, s }; + s++; + y = (v8si) { s, s, s, s, s, s, s, s }; + } + if (y[0] != 2) + abort (); +}
