From 7c46e71d016c86971ac26c6fa38d76482859f296 Mon Sep 17 00:00:00 2001 From: Richard Sandiford Date: Tue, 21 Jan 2020 18:22:38 +0000 Subject: [PATCH] cfgexpand: Update partition size when merging variables cfgexpand sorts variables by decreasing size, so when merging a later variable into an earlier one, there's usually no need to update the merged size. But for poly_int sizes, the sort function just uses a lexicographical comparison of the coefficients, so e.g. 2X+2 comes before 0X+32. Which is bigger depends on the runtime value of X. This patch therefore takes the upper bound of the two sizes, which is conservatively correct for variable-length vectors and a no-op on other targets. It's probably a bad idea to merge fixed-length and variable-length variables in practice, but that's really an optimisation decision. I think we should have this patch as a correctness fix either way. This is easiest to test using the ACLE, but in principle it could happen for autovectorised code too, e.g. when using OpenMP vector variables. It's therefore a regression from GCC 8. 2020-01-22 Richard Sandiford gcc/ * cfgexpand.c (union_stack_vars): Update the size. gcc/testsuite/ * gcc.target/aarch64/sve/acle/general/stack_vars_1.c: New test. --- gcc/ChangeLog | 4 +++ gcc/cfgexpand.c | 3 ++ gcc/testsuite/ChangeLog | 4 +++ .../aarch64/sve/acle/general/stack_vars_1.c | 32 +++++++++++++++++++ 4 files changed, 43 insertions(+) create mode 100644 gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 2f229b92d7a..272968a3396 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,7 @@ +2020-01-22 Richard Sandiford + + * cfgexpand.c (union_stack_vars): Update the size. + 2020-01-22 Richard Biener PR tree-optimization/93381 diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index fb0575b146e..9864e4344d2 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -862,6 +862,9 @@ union_stack_vars (size_t a, size_t b) stack_vars[b].representative = a; stack_vars[a].next = b; + /* Make sure A is big enough to hold B. */ + stack_vars[a].size = upper_bound (stack_vars[a].size, stack_vars[b].size); + /* Update the required alignment of partition A to account for B. */ if (stack_vars[a].alignb < stack_vars[b].alignb) stack_vars[a].alignb = stack_vars[b].alignb; diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3bd6407aac2..d22747b1dfe 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2020-01-22 Richard Sandiford + + * gcc.target/aarch64/sve/acle/general/stack_vars_1.c: New test. + 2020-01-22 Richard Sandiford * gcc.target/aarch64/sve/tls_preserve_1.c: Require tls_native. diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c new file mode 100644 index 00000000000..860fa67fbe6 --- /dev/null +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/stack_vars_1.c @@ -0,0 +1,32 @@ +/* { dg-do run { target aarch64_sve_hw } } */ +/* { dg-additional-options "-O2" } */ + +#include + +struct s { int x[32]; }; + +void __attribute__((noipa)) consume (void *ptr) {} + +void __attribute__((noipa)) +check_var (svint32_t *ptr) +{ + svbool_t pg = svptrue_b8 (); + if (svptest_any (pg, svcmpne (pg, *ptr, svindex_s32 (0, 1)))) + __builtin_abort (); +} + +int +main (void) +{ + svint32_t res = svindex_s32 (0, 1); + { + __SVBool_t pg = svptrue_b8 (); + consume (&pg); + } + { + struct s zeros = { 0 }; + consume (&zeros); + } + check_var (&res); + return 0; +} -- 2.30.2