| Message ID | 1450958105-15859-1-git-send-email-alan.lawrence@arm.com |
|---|---|
| State | New |
| Headers | show |
On 12/24/2015 04:55 AM, Alan Lawrence wrote: > This version changes the test cases to fix failures on some platforms, by > rewriting the initializers so that they aren't pushed out to the constant pool. > > gcc/ChangeLog: > > * tree-ssa-scopedtables.c (avail_expr_hash): Hash MEM_REF and ARRAY_REF > using get_ref_base_and_extent. > (equal_mem_array_ref_p): New. > (hashable_expr_equal_p): Add call to previous. > > gcc/testsuite/ChangeLog: > > * gcc.dg/tree-ssa/ssa-dom-cse-5.c: New. > * gcc.dg/tree-ssa/ssa-dom-cse-6.c: New. > * gcc.dg/tree-ssa/ssa-dom-cse-7.c: New. This is fine. Thanks, Jeff
On Thu, Dec 24, 2015 at 3:55 AM, Alan Lawrence <alan.lawrence@arm.com> wrote: > This version changes the test cases to fix failures on some platforms, by > rewriting the initializers so that they aren't pushed out to the constant pool. > > gcc/ChangeLog: > > * tree-ssa-scopedtables.c (avail_expr_hash): Hash MEM_REF and ARRAY_REF > using get_ref_base_and_extent. > (equal_mem_array_ref_p): New. > (hashable_expr_equal_p): Add call to previous. > This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69352
On 19 January 2016 at 04:05, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Dec 24, 2015 at 3:55 AM, Alan Lawrence <alan.lawrence@arm.com> wrote: >> This version changes the test cases to fix failures on some platforms, by >> rewriting the initializers so that they aren't pushed out to the constant pool. >> >> gcc/ChangeLog: >> >> * tree-ssa-scopedtables.c (avail_expr_hash): Hash MEM_REF and ARRAY_REF >> using get_ref_base_and_extent. >> (equal_mem_array_ref_p): New. >> (hashable_expr_equal_p): Add call to previous. >> > > This caused: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69352 > Hi Alan, This patch also caused regressions on arm-none-linux-gnueabihf with GCC configured as: --with-thumb --with-cpu=cortex-a57 --with-fpu=crypto-neon-fp-armv8 These tests now fail: gcc.dg/torture/pr61742.c -O2 (test for excess errors) gcc.dg/torture/pr61742.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) gcc.dg/torture/pr61742.c -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions (test for excess errors) gcc.dg/torture/pr61742.c -O3 -g (test for excess errors) Christophe
On 19/01/16 09:46, Christophe Lyon wrote: > On 19 January 2016 at 04:05, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Dec 24, 2015 at 3:55 AM, Alan Lawrence <alan.lawrence@arm.com> wrote: >>> This version changes the test cases to fix failures on some platforms, by >>> rewriting the initializers so that they aren't pushed out to the constant pool. >>> >>> gcc/ChangeLog: >>> >>> * tree-ssa-scopedtables.c (avail_expr_hash): Hash MEM_REF and ARRAY_REF >>> using get_ref_base_and_extent. >>> (equal_mem_array_ref_p): New. >>> (hashable_expr_equal_p): Add call to previous. >>> >> >> This caused: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69352 >> > > Hi Alan, > > This patch also caused regressions on arm-none-linux-gnueabihf > with GCC configured as: > --with-thumb --with-cpu=cortex-a57 --with-fpu=crypto-neon-fp-armv8 > > These tests now fail: > gcc.dg/torture/pr61742.c -O2 (test for excess errors) > gcc.dg/torture/pr61742.c -O2 -flto -fno-use-linker-plugin > -flto-partition=none (test for excess errors) > gcc.dg/torture/pr61742.c -O3 -fomit-frame-pointer -funroll-loops > -fpeel-loops -ftracer -finline-functions (test for excess errors) > gcc.dg/torture/pr61742.c -O3 -g (test for excess errors) > Hmm, I still see these passing, both natively on arm-none-linux-gnueabihf and with a cross-build. hf implies --with-float=hard, right? Do you see what the error messages are? Thanks, Alan
On 19 January 2016 at 14:22, Alan Lawrence <alan.lawrence@foss.arm.com> wrote: > On 19/01/16 09:46, Christophe Lyon wrote: >> >> On 19 January 2016 at 04:05, H.J. Lu <hjl.tools@gmail.com> wrote: >>> >>> On Thu, Dec 24, 2015 at 3:55 AM, Alan Lawrence <alan.lawrence@arm.com> >>> wrote: >>>> >>>> This version changes the test cases to fix failures on some platforms, >>>> by >>>> rewriting the initializers so that they aren't pushed out to the >>>> constant pool. >>>> >>>> gcc/ChangeLog: >>>> >>>> * tree-ssa-scopedtables.c (avail_expr_hash): Hash MEM_REF and >>>> ARRAY_REF >>>> using get_ref_base_and_extent. >>>> (equal_mem_array_ref_p): New. >>>> (hashable_expr_equal_p): Add call to previous. >>>> >>> >>> This caused: >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69352 >>> >> >> Hi Alan, >> >> This patch also caused regressions on arm-none-linux-gnueabihf >> with GCC configured as: >> --with-thumb --with-cpu=cortex-a57 --with-fpu=crypto-neon-fp-armv8 >> >> These tests now fail: >> gcc.dg/torture/pr61742.c -O2 (test for excess errors) >> gcc.dg/torture/pr61742.c -O2 -flto -fno-use-linker-plugin >> -flto-partition=none (test for excess errors) >> gcc.dg/torture/pr61742.c -O3 -fomit-frame-pointer -funroll-loops >> -fpeel-loops -ftracer -finline-functions (test for excess errors) >> gcc.dg/torture/pr61742.c -O3 -g (test for excess errors) >> > > Hmm, I still see these passing, both natively on arm-none-linux-gnueabihf > and with a cross-build. hf implies --with-float=hard, right? Do you see what > the error messages are? > Ha! gas complains that "IT blocks containing 32-bit Thumb instructions are deprecated in ARMv8" This is PR67591: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67591 So, not related to your patch in fact. Sorry for the noise. Christophe. > Thanks, Alan >
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-5.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-5.c new file mode 100644 index 0000000..cd38d3e --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-5.c @@ -0,0 +1,18 @@ +/* Test normalization of ARRAY_REF expressions to MEM_REFs in dom. */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fno-tree-fre -fdump-tree-dom2" } */ + +#define N 8 + +int +main (int argc, char **argv) +{ + int a[N]; + for (int i = 0; i < N; i++) + a[i] = 2*i + 1; + int *p = &a[0]; + __builtin_printf ("%d\n", a[argc]); + return *(++p); +} + +/* { dg-final { scan-tree-dump-times "return 3;" 1 "dom2"} } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-6.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-6.c new file mode 100644 index 0000000..002fd81 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-6.c @@ -0,0 +1,20 @@ +/* Test normalization of ARRAY_REF expressions to MEM_REFs in dom. */ +/* { dg-do compile } */ +/* { dg-options "-O1 -fno-tree-fre -fdump-tree-dom2" } */ + +int +main (int argc, char **argv) +{ + union { + int a[4]; + int b[2]; + } u; + u.a[0] = 1; + u.a[1] = 42; + u.a[2] = 3; + u.a[3] = 4; + __builtin_printf ("%d\n", u.a[argc]); + return u.b[1]; +} + +/* { dg-final { scan-tree-dump-times "return 42;" 1 "dom2" } } */ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-7.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-7.c new file mode 100644 index 0000000..151e5d4e --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-dom-cse-7.c @@ -0,0 +1,21 @@ +/* Test normalization of MEM_REF expressions in dom. */ +/* { dg-do compile } */ +/* { dg-options "-O3 -fno-tree-fre -fno-tree-pre -fdump-tree-optimized" } */ + +typedef struct { + int a[8]; +} foo; + +foo f; + +int +test () +{ + foo g; + g.a[0] = 1; g.a[1] = 2; g.a[2] = 3; g.a[3] = 4; + g.a[4] = 5; g.a[5] = 6; g.a[6] = 7; g.a[7] = 8; + f=g; + return f.a[2]; +} + +/* { dg-final { scan-tree-dump-times "return 3;" 1 "optimized" } } */ diff --git a/gcc/tree-ssa-scopedtables.c b/gcc/tree-ssa-scopedtables.c index 6034f79..8df7125 100644 --- a/gcc/tree-ssa-scopedtables.c +++ b/gcc/tree-ssa-scopedtables.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "fold-const.h" #include "tree-eh.h" #include "internal-fn.h" +#include "tree-dfa.h" static bool hashable_expr_equal_p (const struct hashable_expr *, const struct hashable_expr *); @@ -209,11 +210,70 @@ avail_expr_hash (class expr_hash_elt *p) const struct hashable_expr *expr = p->expr (); inchash::hash hstate; + if (expr->kind == EXPR_SINGLE) + { + /* T could potentially be a switch index or a goto dest. */ + tree t = expr->ops.single.rhs; + if (TREE_CODE (t) == MEM_REF || TREE_CODE (t) == ARRAY_REF) + { + /* Make equivalent statements of both these kinds hash together. + Dealing with both MEM_REF and ARRAY_REF allows us not to care + about equivalence with other statements not considered here. */ + bool reverse; + HOST_WIDE_INT offset, size, max_size; + tree base = get_ref_base_and_extent (t, &offset, &size, &max_size, + &reverse); + /* Strictly, we could try to normalize variable-sized accesses too, + but here we just deal with the common case. */ + if (size == max_size) + { + enum tree_code code = MEM_REF; + hstate.add_object (code); + inchash::add_expr (base, hstate); + hstate.add_object (offset); + hstate.add_object (size); + return hstate.end (); + } + } + } + inchash::add_hashable_expr (expr, hstate); return hstate.end (); } +/* Compares trees T0 and T1 to see if they are MEM_REF or ARRAY_REFs equivalent + to each other. (That is, they return the value of the same bit of memory.) + + Return TRUE if the two are so equivalent; FALSE if not (which could still + mean the two are equivalent by other means). */ + +static bool +equal_mem_array_ref_p (tree t0, tree t1) +{ + if (TREE_CODE (t0) != MEM_REF && TREE_CODE (t0) != ARRAY_REF) + return false; + if (TREE_CODE (t1) != MEM_REF && TREE_CODE (t1) != ARRAY_REF) + return false; + + if (!types_compatible_p (TREE_TYPE (t0), TREE_TYPE (t1))) + return false; + bool rev0; + HOST_WIDE_INT off0, sz0, max0; + tree base0 = get_ref_base_and_extent (t0, &off0, &sz0, &max0, &rev0); + + bool rev1; + HOST_WIDE_INT off1, sz1, max1; + tree base1 = get_ref_base_and_extent (t1, &off1, &sz1, &max1, &rev1); + + /* Types were compatible, so these are sanity checks. */ + gcc_assert (sz0 == sz1); + gcc_assert (max0 == max1); + gcc_assert (rev0 == rev1); + + return (off0 == off1) && operand_equal_p (base0, base1, 0); +} + /* Compare two hashable_expr structures for equivalence. They are considered equivalent when the expressions they denote must necessarily be equal. The logic is intended to follow that of @@ -246,9 +306,10 @@ hashable_expr_equal_p (const struct hashable_expr *expr0, switch (expr0->kind) { case EXPR_SINGLE: - return operand_equal_p (expr0->ops.single.rhs, - expr1->ops.single.rhs, 0); - + return equal_mem_array_ref_p (expr0->ops.single.rhs, + expr1->ops.single.rhs) + || operand_equal_p (expr0->ops.single.rhs, + expr1->ops.single.rhs, 0); case EXPR_UNARY: if (expr0->ops.unary.op != expr1->ops.unary.op) return false;