On Sun, Jan 15, 2017 at 10:34 AM, Jeff Law <l...@redhat.com> wrote:
>
> At one time I know I had the max_size == size test in valid_ao_ref_for_dse.
> But it got lost at some point. This is what caused the Ada failure.
>
> Technically it'd be OK for the potentially dead store to have a variable
> size as long as the later stores covered the entire range of the potentially
> dead store. I doubt this happens enough to be worth checking.
>
> The ppc64 big endian failures were more interesting. We had this in the IL:
>
> memmove (dst, src, 0)
>
> The trimming code assumes that there's at least one live byte in the store,
> which obviously isn't the case here. The net result is we compute an
> incorrect trim and the copy goes wild with incorrect addresses and lengths.
> This is trivial to fix by validating that the store has a nonzero length.
>
> I was a bit curious how often this happened in practice because such a call
> is trivially dead. ~80 during a bootstrap and a few dozen in the testsuite.
> Given how trivial it is to detect and optimize, this patch includes removal
> of such calls. This hunk makes the check for zero size in
> valid_ao_ref_for_dse redundant, but I'd like to keep the check -- if we add
> more builtin support without filtering zero size we'd regress again.
Interesting - we do fold memset (..., 0) away so this means we either
have an unfolded
memset stmt in the IL before DSE.
> I wanted to get this in tonight given the Ada and ppc64 breakage so I didn't
> do testcase reductions for the ppc test or the zero length optimization
> (acats covers the Ada breakage). I'll get testcases done Monday. I'll
> also finish generating suitable baselines for ppc64-linux-gnu over the rest
> of the weekend to verify if this fixes all the ppc64 regressions or just a
> subset of them.
>
> Bootstrapped on x86_64-linux-gnu and ppc64-linux-gnu. Regression tested on
> x86_64-linux-gnu. Also verified the ppc64 testresults are improved
> (anything where sel-sched was faulting ought to be fixed now, maybe
> others). Installing on the trunk.
>
> Jeff
>
> commit 04c5d9cab65e2f5b219f9610621bea06173b9cb8
> Author: Jeff Law <l...@redhat.com>
> Date: Sun Jan 15 01:37:32 2017 -0700
>
> PR tree-optimization/79090
> * tree-ssa-dse.c (valid_ao_ref_for_dse): Reject zero length and
> variable length stores.
> (compute_trims): Delete dead assignment to *trim_tail.
> (dse_dom_walker::dse_optimize_stmt): Optimize mem* calls with
> zero length.
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 746f6af..36982c6 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,12 @@
> +2017-01-14 Jeff Law <l...@redhat.com>
> +
> + PR tree-optimization/79090
> + * tree-ssa-dse.c (valid_ao_ref_for_dse): Reject zero length and
> + variable length stores.
> + (compute_trims): Delete dead assignment to *trim_tail.
> + (dse_dom_walker::dse_optimize_stmt): Optimize mem* calls with
> + zero length.
> +
> 2017-01-14 Bernd Schmidt <bschm...@redhat.com>
>
> PR rtl-optimization/78626
> diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
> index 2e6f8ff..65dcb12 100644
> --- a/gcc/tree-ssa-dse.c
> +++ b/gcc/tree-ssa-dse.c
> @@ -129,6 +129,8 @@ valid_ao_ref_for_dse (ao_ref *ref)
> {
> return (ao_ref_base (ref)
> && ref->max_size != -1
> + && ref->size != 0
> + && ref->max_size == ref->size
> && (ref->offset % BITS_PER_UNIT) == 0
> && (ref->size % BITS_PER_UNIT) == 0
> && (ref->size != -1));
> @@ -221,7 +223,6 @@ compute_trims (ao_ref *ref, sbitmap live, int
> *trim_head, int *trim_tail)
> the REF to compute the trims. */
>
> /* Now identify how much, if any of the tail we can chop off. */
> - *trim_tail = 0;
> int last_orig = (ref->size / BITS_PER_UNIT) - 1;
> int last_live = bitmap_last_set_bit (live);
> *trim_tail = (last_orig - last_live) & ~0x1;
> @@ -700,6 +701,16 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator
> *gsi)
> case BUILT_IN_MEMMOVE:
> case BUILT_IN_MEMSET:
> {
> + /* Occasionally calls with an explicit length of zero
> + show up in the IL. It's pointless to do analysis
> + on them, they're trivially dead. */
> + tree size = gimple_call_arg (stmt, 2);
> + if (integer_zerop (size))
> + {
> + delete_dead_call (gsi);
> + return;
> + }
> +
> gimple *use_stmt;
> enum dse_store_status store_status;
> m_byte_tracking_enabled
>
