| Message ID | nycvar.YFH.7.76.1909051035290.5566@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR91657 (and likely dups) | expand |
On Thu, 5 Sep 2019, Richard Biener wrote: > > I've made an error with the gcse-after-reload patch in disabling > record_last_mem_set_info, but only for the first function so > testing didn't reveal it. > > The following fixes this, avoding a few more allocations when > not needing transp computation and checking a variable which > is actually initialized at the point it is queried. > > Bootstrap & regtest running on x86_64-unknown-linux-gnu > both with transp compute forced to be off and not. > > Will apply once testing succeeds. Going with the following simpler change, the previous showed some other issues. Bootstrapped on x86_64-unknown-linux-gnu with -fgcse-after-reload enabled and transp forced off sofar. More testing in progress. (I realized -O2 doesn't enable -fgcse-after-reload) Richard. 2019-09-05 Richard Biener <rguenther@suse.de> PR rtl-optimization/91656 * postreload-gcse.c (record_last_mem_set_info): Revert addition of early out. * gcc.dg/torture/pr91656-1.c: New testcase. * gcc.dg/torture/pr91656-2.c: Likewise. * gcc.dg/torture/pr91656-3.c: Likewise. Index: gcc/testsuite/gcc.dg/torture/pr91656-1.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-1.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-1.c (working copy) @@ -0,0 +1,24 @@ +/* { dg-do run } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +int a, b, c, d, e; + +static __attribute__ ((__noipa__)) +int foo (int i) +{ + __builtin_memmove (&i, &e, 1); + if (a > 0) + i /= e; + e /= 5; + b = 0; + return i + c + d + 5; +} + +int +main (void) +{ + int x = foo (4); + if (x != 5) + __builtin_abort (); + return 0; +} Index: gcc/testsuite/gcc.dg/torture/pr91656-2.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-2.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-2.c (working copy) @@ -0,0 +1,27 @@ +/* { dg-do run { target int128 } } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +typedef unsigned char u8; +typedef unsigned short u16; +typedef unsigned int u32; +typedef unsigned long long u64; +int a, b, c; +__int128 e; +int +d (u16 g) +{ + u64 f = __builtin_bswap64 (c); + f = g == a; + __builtin_memmove (&f, &e, 1); + e >>= b; + return a + f; +} + +int +main (void) +{ + __int128 x = d (0); + if (x != 0) + __builtin_abort (); + return 0; +} Index: gcc/testsuite/gcc.dg/torture/pr91656-3.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-3.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-3.c (working copy) @@ -0,0 +1,25 @@ +/* { dg-do run { target int128 } } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +typedef unsigned char u8; +typedef unsigned short u16; +typedef unsigned int u32; +typedef unsigned long long u64; +int a, b, c; +int +d (u16 e, u64 f) +{ + b |= e; + __builtin_memset (&f, e, 2); + a = (u16) - e >= 2 ? : __builtin_popcountll (f); + return a + c; +} + +int +main (void) +{ + __int128 x = d (~0, 0); + if (x != 16) + __builtin_abort (); + return 0; +} Index: gcc/postreload-gcse.c =================================================================== --- gcc/postreload-gcse.c (revision 275399) +++ gcc/postreload-gcse.c (working copy) @@ -696,9 +696,6 @@ record_last_reg_set_info_regno (rtx_insn static void record_last_mem_set_info (rtx_insn *insn) { - if (!transp) - return; - struct modifies_mem *list_entry; list_entry = (struct modifies_mem *) obstack_alloc (&modifies_mem_obstack,
Index: gcc/postreload-gcse.c =================================================================== --- gcc/postreload-gcse.c (revision 275399) +++ gcc/postreload-gcse.c (working copy) @@ -231,8 +231,8 @@ static sbitmap *transp; /* Helpers for memory allocation/freeing. */ -static void alloc_mem (void); -static void free_mem (void); +static void alloc_mem (bool); +static void free_mem (bool); /* Support for hash table construction and transformations. */ static bool oprs_unchanged_p (rtx, rtx_insn *, bool); @@ -274,7 +274,7 @@ static void eliminate_partially_redundan tracking tables. */ static void -alloc_mem (void) +alloc_mem (bool do_transp) { int i; basic_block bb; @@ -309,6 +309,9 @@ alloc_mem (void) in the current block. */ reg_avail_info = (int *) xmalloc (FIRST_PSEUDO_REGISTER * sizeof (int)); + if (!do_transp) + return; + /* Put a dummy modifies_mem object on the modifies_mem_obstack, so we can roll it back in reset_opr_set_tables. */ modifies_mem_obstack_bottom = @@ -328,7 +331,7 @@ alloc_mem (void) /* Free memory allocated by alloc_mem. */ static void -free_mem (void) +free_mem (bool do_transp) { free (uid_cuid); @@ -338,6 +341,10 @@ free_mem (void) obstack_free (&expr_obstack, NULL); obstack_free (&occr_obstack, NULL); obstack_free (&unoccr_obstack, NULL); + + if (!do_transp) + return; + obstack_free (&modifies_mem_obstack, NULL); unsigned i; @@ -351,8 +358,11 @@ free_mem (void) BITMAP_FREE (blocks_with_calls); BITMAP_FREE (modify_mem_list_set); free (reg_avail_info); + reg_avail_info = NULL; free (modify_mem_list); + modify_mem_list = NULL; free (canon_modify_mem_list); + canon_modify_mem_list = NULL; } @@ -696,7 +706,7 @@ record_last_reg_set_info_regno (rtx_insn static void record_last_mem_set_info (rtx_insn *insn) { - if (!transp) + if (!modify_mem_list) return; struct modifies_mem *list_entry; @@ -1370,7 +1380,7 @@ gcse_after_reload_main (rtx f ATTRIBUTE_ /* Allocate memory for this pass. Also computes and initializes the insns' CUIDs. */ - alloc_mem (); + alloc_mem (do_transp); /* We need alias analysis. */ init_alias_analysis (); @@ -1422,7 +1432,7 @@ gcse_after_reload_main (rtx f ATTRIBUTE_ /* We are finished with alias. */ end_alias_analysis (); - free_mem (); + free_mem (do_transp); } Index: gcc/testsuite/gcc.dg/torture/pr91656-1.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-1.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-1.c (working copy) @@ -0,0 +1,24 @@ +/* { dg-do run } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +int a, b, c, d, e; + +static __attribute__ ((__noipa__)) +int foo (int i) +{ + __builtin_memmove (&i, &e, 1); + if (a > 0) + i /= e; + e /= 5; + b = 0; + return i + c + d + 5; +} + +int +main (void) +{ + int x = foo (4); + if (x != 5) + __builtin_abort (); + return 0; +} Index: gcc/testsuite/gcc.dg/torture/pr91656-2.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-2.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-2.c (working copy) @@ -0,0 +1,27 @@ +/* { dg-do run { target int128 } } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +typedef unsigned char u8; +typedef unsigned short u16; +typedef unsigned int u32; +typedef unsigned long long u64; +int a, b, c; +__int128 e; +int +d (u16 g) +{ + u64 f = __builtin_bswap64 (c); + f = g == a; + __builtin_memmove (&f, &e, 1); + e >>= b; + return a + f; +} + +int +main (void) +{ + __int128 x = d (0); + if (x != 0) + __builtin_abort (); + return 0; +} Index: gcc/testsuite/gcc.dg/torture/pr91656-3.c =================================================================== --- gcc/testsuite/gcc.dg/torture/pr91656-3.c (nonexistent) +++ gcc/testsuite/gcc.dg/torture/pr91656-3.c (working copy) @@ -0,0 +1,25 @@ +/* { dg-do run { target int128 } } */ +/* { dg-additional-options "-fgcse-after-reload" } */ + +typedef unsigned char u8; +typedef unsigned short u16; +typedef unsigned int u32; +typedef unsigned long long u64; +int a, b, c; +int +d (u16 e, u64 f) +{ + b |= e; + __builtin_memset (&f, e, 2); + a = (u16) - e >= 2 ? : __builtin_popcountll (f); + return a + c; +} + +int +main (void) +{ + __int128 x = d (~0, 0); + if (x != 16) + __builtin_abort (); + return 0; +}