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); } } 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)
