On Mon, Jun 1, 2026 at 7:35 AM Dylan Rees <[email protected]> wrote: > > The fix to improve the folding of low-to-highpart builtins resulted > in redundant duplication instructions. The compiler was placing the > uniform vector arguments for low/highpart intrinsics in separate > registers despite them duplicating the same scalar value. This turns > out to be a result of the ordering of the duplication instructions, > when a narrow duplication of a scalar value appears before a wider > duplication of that same scalar the compiler is unable to link them > together into a single operation. > > It is desirable in this case to create a single wider vector and > take the lower half for narrower uses of the same vector, since we > can then use a single load instruction. This patch fixes this logic > within CSE by matching vector duplicates during 'cse_prescan', if > they are duplicating the same pseudo into vectors of different > widths but the same element size. In 'cse_main' these matches are > processed and the wider vector duplicate insn is emitted before > all matching duplications, narrower duplicate insns will > be converted to a SUBREG of this initial duplication and duplicates > of the same width will reuse it. > > Two test cases were updated to be more flexible with regexs since > hardcoded values were causing unnecessary failures despite the > code being logically correct.
I am curious if this makes part of the x86_cse (the remove_redundant_vector part) not do as much. I like the idea of a generic pass to handle this cases rather than a target specific one. I have not checked but does this handle constants and not just vec_duplicates? E.g. 0 or -1 would seem to good to handle too. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99639 is another example where constants can show up. Thanks, Andrea > > gcc/ChangeLog: > > * cse.cc (struct cse_vec_duplicate_match): New struct. > (struct cse_basic_block_data): New member added. > (cse_prescan_cache_vec_dup): New function to cache any vector > duplications and check for potential matches. > (cse_prescan_path): Extended pre_scan loop to cache vector > duplication matches + New loop added to assign a list of > matches to a block of data inside new struct member. > (cse_main): New loop to emit wider vector duplication earlier > and update narrower vector duplicate to SUBREG insn. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/simd/fold_to_highpart_7.c: > Extended test. > * gcc.target/i386/pr81501-9a.c: Updated test. > * gcc.target/i386/pr81501-9b.c: Updated test. > --- > gcc/cse.cc | 239 +++++++++++++++++- > .../aarch64/simd/fold_to_highpart_7.c | 8 +- > gcc/testsuite/gcc.target/i386/pr81501-9a.c | 4 +- > gcc/testsuite/gcc.target/i386/pr81501-9b.c | 2 +- > 4 files changed, 244 insertions(+), 9 deletions(-) > > diff --git a/gcc/cse.cc b/gcc/cse.cc > index c1e0cc2167c..6c0e5d879bc 100644 > --- a/gcc/cse.cc > +++ b/gcc/cse.cc > @@ -483,6 +483,39 @@ struct branch_path > basic_block bb; > }; > > +/* This data describes a pair of vec_duplicates in the same BB, which > duplicate > + the same pseudo to different vector lengths. The same structure is also > + used while prescanning a basic block as a temporary cache entry. */ > + > +struct cse_vec_duplicate_match > +{ > + basic_block bb; > + machine_mode widest_mode; > + rtx scalar; > + rtx_insn *first_insn; > + rtx_insn *widest_insn; > + auto_vec<rtx_insn *> related_dups; > + > + cse_vec_duplicate_match () = default; > + > + cse_vec_duplicate_match (basic_block bb_, machine_mode widest_mode_, > + rtx scalar_, rtx_insn *first_insn_, > + rtx_insn *widest_insn_) > + : bb (bb_), widest_mode (widest_mode_), scalar (scalar_), > + first_insn (first_insn_), widest_insn (widest_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) > + { > + unsigned int i; > + rtx_insn *insn; > + FOR_EACH_VEC_ELT (other.related_dups, i, insn) > + related_dups.safe_push (insn); > + } > +}; > + > /* This data describes a block that will be processed by > cse_extended_basic_block. */ > > @@ -494,6 +527,10 @@ struct cse_basic_block_data > int path_size; > /* Current path, indicating which basic_blocks will be processed. */ > struct branch_path *path; > + /* vec_duplicate sources seen in the current BB while prescanning. */ > + auto_vec<cse_vec_duplicate_match, 8> vec_duplicate_cache; > + /* Syntactic vec_duplicate matches found in the same BB while prescanning. > */ > + auto_vec<cse_vec_duplicate_match, 8> vec_duplicate_matches; > }; > > > @@ -545,6 +582,8 @@ static rtx equiv_constant (rtx); > static void record_jump_equiv (rtx_insn *, bool); > static void record_jump_cond (enum rtx_code, machine_mode, rtx, rtx); > static void cse_insn (rtx_insn *); > +static void cse_prescan_cache_vec_dup (struct cse_basic_block_data *, > + basic_block, rtx_insn *, rtx); > static void cse_prescan_path (struct cse_basic_block_data *); > static void invalidate_from_clobbers (rtx_insn *); > static void invalidate_from_sets_and_clobbers (rtx_insn *); > @@ -6527,9 +6566,63 @@ have_eh_succ_edges (basic_block bb) > return false; > } > > +/* Record vec_duplicate match for SET in the same basic block, if any. */ > +static void > +cse_prescan_cache_vec_dup (struct cse_basic_block_data *data, basic_block bb, > + rtx_insn *insn, rtx set) > +{ > + rtx src = SET_SRC (set); > + rtx dest = SET_DEST (set); > + machine_mode mode = GET_MODE (src); > + rtx scalar; > + > + /* Limit matching to duplicates of the same pseudo register, or an exact > + SUBREG of such a pseudo. */ > + if (!vec_duplicate_p (src, &scalar) > + || !(REG_P (scalar) > + ? REGNO (scalar) >= FIRST_PSEUDO_REGISTER > + : (GET_CODE (scalar) == SUBREG > + && REG_P (SUBREG_REG (scalar)) > + && REGNO (SUBREG_REG (scalar)) >= FIRST_PSEUDO_REGISTER))) > + return; > + > + /* Check for matching cached vec_duplicates and save the match if found. > */ > + for (unsigned int i = 0; i < data->vec_duplicate_cache.length (); i++) > + { > + cse_vec_duplicate_match &entry = data->vec_duplicate_cache[i]; > + > + /* Create a match with existing cache entry if both scalar register > + pseudos are matching and the new vec_duplicate mode is wider. */ > + if (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) > + && REG_P (dest)) > + { > + entry.widest_mode = mode; > + entry.widest_insn = insn; > + entry.related_dups.safe_push (insn); > + return; > + } > + else if (rtx_equal_p (entry.scalar, scalar) > + && known_le (GET_MODE_SIZE (mode), > + GET_MODE_SIZE (entry.widest_mode)) > + && GET_MODE_INNER (mode) == GET_MODE_INNER (entry.widest_mode)) > + { > + 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); > + new_entry.related_dups.safe_push (insn); > + data->vec_duplicate_cache.safe_push (new_entry); > +} > + > > /* Scan to the end of the path described by DATA. Return an estimate of > - the total number of SETs of all insns in the path. */ > + the total number of SETs of all insns in the path. Also record any > + matching vec_duplicate SETs of the same scalar value for different modes. > */ > > static void > cse_prescan_path (struct cse_basic_block_data *data) > @@ -6538,6 +6631,8 @@ cse_prescan_path (struct cse_basic_block_data *data) > int path_size = data->path_size; > int path_entry; > > + data->vec_duplicate_matches.truncate (0); > + > /* Scan to end of each basic block in the path. */ > for (path_entry = 0; path_entry < path_size; path_entry++) > { > @@ -6545,18 +6640,47 @@ cse_prescan_path (struct cse_basic_block_data *data) > rtx_insn *insn; > > bb = data->path[path_entry].bb; > + data->vec_duplicate_cache.truncate (0); > > FOR_BB_INSNS (bb, insn) > { > if (!INSN_P (insn)) > continue; > > + rtx pattern = PATTERN (insn); > + > /* A PARALLEL can have lots of SETs in it, > especially if it is really an ASM_OPERANDS. */ > - if (GET_CODE (PATTERN (insn)) == PARALLEL) > - nsets += XVECLEN (PATTERN (insn), 0); > + if (GET_CODE (pattern) == PARALLEL) > + { > + int len = XVECLEN (pattern, 0); > + > + nsets += len; > + for (int i = 0; i < len; i++) > + { > + rtx elt = XVECEXP (pattern, 0, i); > + if (GET_CODE (elt) == SET) > + cse_prescan_cache_vec_dup (data, bb, insn, elt); > + } > + } > else > - nsets += 1; > + { > + if (GET_CODE (pattern) == SET) > + cse_prescan_cache_vec_dup (data, bb, insn, pattern); > + nsets += 1; > + } > + } > + > + /* Record any vec_duplicate matches from this basic block. */ > + for (unsigned int i = 0; i < data->vec_duplicate_cache.length (); i++) > + { > + cse_vec_duplicate_match &entry = data->vec_duplicate_cache[i]; > + > + /* If we recorded no wider vec_duplicate match then skip. */ > + if (entry.widest_insn == NULL) > + continue; > + > + data->vec_duplicate_matches.safe_push (entry); > } > } > > @@ -6803,6 +6927,113 @@ 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 (unsigned int i = 0; > + i < ebb_data.vec_duplicate_matches.length (); > + i++) > + { > + cse_vec_duplicate_match &match = > ebb_data.vec_duplicate_matches[i]; > + > + 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; > + > + /* 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; > + unsigned int j; > + rtx_insn *dup_insn; > + > + FOR_EACH_VEC_ELT (match.related_dups, j, dup_insn) > + { > + rtx dup_set; > + rtx dup_src; > + machine_mode dup_mode; > + > + 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. */ > + for (rtx_insn *scan = NEXT_INSN (match.widest_insn); > + scan != NULL_RTX; scan = NEXT_INSN (scan)) > + if (scan == dup_insn) > + { > + if (reg_set_between_p (wide_reg, > + match.widest_insn, > + dup_insn)) > + continue; > + 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 = copy_rtx (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 (!validate_change (dup_insn, &SET_SRC (dup_set), dup_src, > 1)) > + { > + abort_changes = true; > + break; > + } > + } > + > + if (abort_changes) > + { > + cancel_changes (prev_changes); > + continue; > + } > + > + if (num_changes_pending () == prev_changes) > + continue; > + > + if (!apply_change_group ()) > + continue; > + > + reorder_insns (match.widest_insn, match.widest_insn, > insert_after); > + > + FOR_EACH_VEC_ELT (match.related_dups, j, dup_insn) > + if (dup_insn != match.widest_insn) > + df_insn_rescan (dup_insn); > + } > + > /* Get a reasonable estimate for the maximum number of qty's > needed for this path. For this, we take the number of sets > and multiply that by MAX_RECOG_OPERANDS. */ > diff --git a/gcc/testsuite/gcc.target/aarch64/simd/fold_to_highpart_7.c > b/gcc/testsuite/gcc.target/aarch64/simd/fold_to_highpart_7.c > index 941dab3e9b9..ae7c368da8c 100644 > --- a/gcc/testsuite/gcc.target/aarch64/simd/fold_to_highpart_7.c > +++ b/gcc/testsuite/gcc.target/aarch64/simd/fold_to_highpart_7.c > @@ -4,7 +4,9 @@ > #include <arm_neon.h> > > /* We should fold to the highpart builtin when the multiplying the highpart > of a > - 128b vector with a uniform vector which can be widened. > + 128b vector with a uniform vector. Also the uniform vector should be > loaded > + as a single wider vector duplicate load (ld1r) using the lower half for > the > + lowpart builtin and the full vector duplicate for the highpart builtin. > > Use vdup_n_u8 to create an 8x8 splat vector. */ > > @@ -15,4 +17,6 @@ uint16x8_t foo(uint8_t *a, uint8_t *b) { > } > > /* { dg-final { scan-assembler {mull2\t} } } */ > -/* { dg-final { scan-assembler-not {mull\t} } } */ > \ No newline at end of file > +/* { dg-final { scan-assembler-not {mull\t} } } */ > +/* { dg-final { scan-assembler {\tld1r\t} } } */ > +/* { dg-final { scan-assembler-not {\tdup\tv} } } */ > diff --git a/gcc/testsuite/gcc.target/i386/pr81501-9a.c > b/gcc/testsuite/gcc.target/i386/pr81501-9a.c > index 66a2768a22c..ea7772dbb21 100644 > --- a/gcc/testsuite/gcc.target/i386/pr81501-9a.c > +++ b/gcc/testsuite/gcc.target/i386/pr81501-9a.c > @@ -7,10 +7,10 @@ > **foo: > **.LFB[0-9]+: > **... > -** vpbroadcastb %edi, %zmm0 > -**... > ** call __tls_get_addr@PLT > **... > +** vpbroadcastb %(edi|ebx), %zmm\d > +**... > */ > > #include <immintrin.h> > diff --git a/gcc/testsuite/gcc.target/i386/pr81501-9b.c > b/gcc/testsuite/gcc.target/i386/pr81501-9b.c > index 711b177bc1e..bd1b65d3e87 100644 > --- a/gcc/testsuite/gcc.target/i386/pr81501-9b.c > +++ b/gcc/testsuite/gcc.target/i386/pr81501-9b.c > @@ -8,7 +8,7 @@ > **foo: > **.LFB[0-9]+: > **... > -** vpbroadcastb %edi, %zmm0 > +** vpbroadcastb %(edi|r\d+d), %zmm\d > **... > ** lea(l|q) var@TLSDESC\(%rip\), %(e|r)ax > **... > -- > 2.43.0 >
