https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126598
Bug ID: 126598
Summary: Wrong code with vectorisation and
vectorizable_with_step_bound_p
Product: gcc
Version: 17.0
Status: UNCONFIRMED
Keywords: wrong-code
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: ktkachov at gcc dot gnu.org
Target Milestone: ---
/* Candidate: tree-vect-data-refs.cc:4189, vectorizable_with_step_bound_p.
THE DEFECT
vectorizable_with_step_bound_p sorts the two data references so that
init_a <= init_b and then computes
*lower_bound_out = init_b + vect_get_scalar_dr_size (dr_info_b) - init_a;
Its own comment asks for "the bytes spanned by the combination of the two
accesses", which is max (init_a + size_a, init_b + size_b) - init_a. The
two differ exactly when the lower-addressed access is the larger one.
Let d = init_b - init_a. A in iteration k overlaps B in iteration k + m
iff -(d + size_b) < m * step < size_a - d. The emitted bound
abs (step) >= d + size_b only excludes m <= -1. Every m >= 1 overlap with
step in [d + size_b, size_a - d) survives. The guard just above only calls
vect_preserves_scalar_order_p, which returns true unconditionally for two
ungrouped statements and in any case only rules out m == 0.
vect_prune_runtime_alias_test_list then drops the alias check altogether
and emits nothing but the lower-bound test.
Here A is the 8-byte store at DR_INIT 0 and B is the 4-byte load at
DR_INIT 1, so d = 1, size_a = 8, size_b = 4. Emitted bound 5, safe bound 8.
-fdump-tree-vect-details shows the transform firing:
note: no alias between MEM <unsigned int> [(char * {ref-all})_5] and
MEM <unsigned long> [(char * {ref-all})_10] when the step
(ssizetype) stride_18(D) is outside (-5, 5)
note: improved number of alias checks from 1 to 0
optimized: loop vectorized using 16 byte vectors and unroll factor 4
optimized: loop versioned for vectorization because of possible aliasing
Strides 5 and 6 pass the emitted check but do overlap, and they are exactly
the two that give a wrong answer:
stride 5: vec=206536 ref=845086
stride 6: vec=5632 ref=16357
The load statement must come before the store statement. With the store
first the compiler emits the same wrong bound (-5, 5) but the program still
gets the right answer, because only the m >= 1 direction is unprotected.
*/
#define BODY \
unsigned t; \
__builtin_memcpy (&t, p + i * stride + 1, sizeof (t)); \
t = t * 3u; \
t = t + 7u; \
t = t ^ 5u; \
t = t * 11u; \
sum += t; \
__builtin_memcpy (p + i * stride, &v, sizeof (v));
__attribute__((noipa)) unsigned
f (char *p, long stride, int n, long long v)
{
unsigned sum = 0;
for (int i = 0; i < n; ++i)
{
BODY
}
return sum;
}
__attribute__((noipa)) unsigned
ref (char *p, long stride, int n, long long v)
{
unsigned sum = 0;
for (int i = 0; i < n; ++i)
{
BODY
__asm__ volatile ("" ::: "memory");
}
return sum;
}
static char a[2048], b[2048];
int
main (void)
{
long long v = 0x0102030405060708LL;
for (long stride = 1; stride <= 12; ++stride)
{
__builtin_memset (a, 0, sizeof (a));
__builtin_memset (b, 0, sizeof (b));
unsigned s1 = f (a, stride, 100, v);
unsigned s2 = ref (b, stride, 100, v);
if (s1 != s2 || __builtin_memcmp (a, b, sizeof (a)) != 0)
__builtin_abort ();
}
return 0;
}
aborts on aarch64 at -O3 as far back as I could check and passes without
vectorisation