On Mon, Jun 01, 2026 at 12:34:15PM -0700, Andrew Pinski wrote:
> 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’m not entirely sure about this either, but until recently there were
some x86_cse regression failures. They disappeared, so I also wonder
whether there is some overlap between the two.
> 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.
No, this patch doesn’t handle constants, only vector duplicates.
However, I agree that this structure could be extended to address those
cases as well.
>
> Thanks,
> Andrea
Thanks,
Dylan
>
> >
> > 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
> >
