On Mon, Jun 15, 2026 at 7:29 AM <[email protected]> wrote:
>
> From: Kyrylo Tkachov <[email protected]>
>
> early_ra::allocate_colors marks the FPRs occupied by a color with
>
> m_allocated_fprs |= ((1U << color->group->size) - 1) << best;
>
> When a color group spans the whole register file (size == 32), as can
> happen for a heavily unrolled, vectorized loop, "1U << 32" is undefined
> and evaluates to 1 sometimes, so the expression sets no bits at all.
> The 32 FPRs of the group are therefore not recorded as allocated.
> Subsequent colors (and broaden_colors) then reuse those registers, which
> breaks the invariant that distinct colors receive disjoint FPRs.
>
> In PR125795 this let the loop-invariant TBL permute index, which is live
> across the whole loop, share v28 with the LD2 tuple destinations, so the
> index was clobbered mid-loop and the loop produced wrong results.
>
> Fix this by building the mask with a loop, which is well-defined for any
> group size in [1, 32] (the candidate search already guarantees
> best + size <= 32, which the patch also asserts). When the full-width
> group can no longer be hidden, allocate_colors correctly fails to find a
> register for the other color and the region is left to the real register
> allocator, matching -mearly-ra=none.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> I'll push it to trunk tomorrow if no objections.
> Thanks,
> Kyrill
>
> Signed-off-by: Kyrylo Tkachov <[email protected]>
>
> gcc/ChangeLog:
>
> PR target/125795
> * config/aarch64/aarch64-early-ra.cc (early_ra::allocate_colors):
> Build the allocated-FPR mask with a loop instead of
> "((1U << size) - 1) << best".
>
> gcc/testsuite/ChangeLog:
>
> PR target/125795
> * gcc.target/aarch64/pr125795.c: New test.
> ---
> gcc/config/aarch64/aarch64-early-ra.cc | 9 +++-
> gcc/testsuite/gcc.target/aarch64/pr125795.c | 47 +++++++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.target/aarch64/pr125795.c
>
> diff --git a/gcc/config/aarch64/aarch64-early-ra.cc
> b/gcc/config/aarch64/aarch64-early-ra.cc
> index 24f0e1bdffb..243017e4ac1 100644
> --- a/gcc/config/aarch64/aarch64-early-ra.cc
> +++ b/gcc/config/aarch64/aarch64-early-ra.cc
> @@ -3038,7 +3038,14 @@ early_ra::allocate_colors ()
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, " Allocating [v%d:v%d] to color %d\n",
> best, best + color->group->size - 1, color->id);
> - m_allocated_fprs |= ((1U << color->group->size) - 1) << best;
> + // Mark the COLOR's FPRs as allocated. Build the mask with a loop
> + // rather than ((1U << size) - 1), since a full-width color can have
> + // size == 32 and "1U << 32" would be undefined. The candidate
> + // search above guarantees best + size <= 32, so each shift is in
> + // range.
> + gcc_assert (best + color->group->size <= 32);
> + for (unsigned int i = 0; i < color->group->size; ++i)
> + m_allocated_fprs |= 1U << (best + i);
Why not just:
gcc_assert (best + color->group->size <= 32);
if (best + color->group->size == 32)
m_allocated_fprs = -1U;
else
m_allocated_fprs |= ((1U << color->group->size) - 1) << best;
The loop seems a bit much here.
Thanks,
Andrea
> }
> }
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr125795.c
> b/gcc/testsuite/gcc.target/aarch64/pr125795.c
> new file mode 100644
> index 00000000000..9f5c6a32b5d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr125795.c
> @@ -0,0 +1,47 @@
> +/* PR target/125795 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -funroll-loops" } */
> +
> +/* This loop is vectorized with LD2 (deinterleave) and TBL (lane reversal),
> + then unrolled. The unrolled body builds a full-width (32 FPR) early-ra
> + color group, which used to trigger a "1U << 32" undefined shift in
> + early_ra::allocate_colors. As a result the long-lived TBL permute index
> + was wrongly allocated to the same FPR as an LD2 tuple destination and got
> + clobbered, so the result was miscomputed. */
> +
> +extern void abort (void);
> +
> +#define N 256
> +
> +static int __attribute__ ((noipa))
> +foo (void)
> +{
> + int k[N];
> + int k2[N];
> + int i;
> +
> + for (i = 0; i < N; i++)
> + k2[i] = i;
> +
> + for (i = 0; i < 64; i++)
> + {
> + k[i] = k2[2 * i + 0];
> + k[255 - i] = k2[2 * i + 1];
> + k[64 + i] = k2[2 * i + 128];
> + k[191 - i] = k2[2 * i + 129];
> + }
> +
> + for (i = 0; i < N; i++)
> + if (k[i] != ((i < 128) ? (2 * i) : (511 - 2 * i)))
> + return 1;
> +
> + return 0;
> +}
> +
> +int
> +main (void)
> +{
> + if (foo ())
> + abort ();
> + return 0;
> +}
> --
> 2.50.1 (Apple Git-155)
>
