Re: [PATCH] Fix superfluous insertion by load-PRE with PR21485

["Daniel Berlin" <dberlin at dberlin dot org>]

I've been half following that bug.
Should I be looking into this, or assume it's a target dependent
problem, and go back to speeding up SCCVN? :)


On Sat, Oct 4, 2008 at 10:18 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> In PR21485 load-PRE does not properly detect the full redundancy on
> the k+1 path and so does an unnecessary insertion.  This is because
> the types involved are not pointer equivalent (as we check now), but
> are compatible, what is what really matters.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>
> No PR tree-optimization/21485 here though, as with the removed insertion
> the testcase degrades somewhat more.  But as I learned from Intel recently
> the Core microarchitecture has some "interesting" properties regarding
> to runtime behavior of small loops with many branches.  Which means the
> problem is somewhere in the target dependent parts.
>
> Richard.
>
> 2008-10-04  Richard Guenther  <rguenther@suse.de>
>
>        * tree-ssa-sccvn.c (vn_reference_op_eq): Use types_compatible_p
>        instead of pointer equality.
>        (vn_nary_op_eq): Likewise.
>
>        * gcc.dg/tree-ssa/ssa-pre-21.c: New testcase.
>
> Index: gcc/tree-ssa-sccvn.c
> ===================================================================
> *** gcc/tree-ssa-sccvn.c        (revision 140862)
> --- gcc/tree-ssa-sccvn.c        (working copy)
> *************** vn_reference_op_eq (const void *p1, cons
> *** 387,393 ****
>    const_vn_reference_op_t const vro1 = (const_vn_reference_op_t) p1;
>    const_vn_reference_op_t const vro2 = (const_vn_reference_op_t) p2;
>    return vro1->opcode == vro2->opcode
> !     && vro1->type == vro2->type
>      && expressions_equal_p (vro1->op0, vro2->op0)
>      && expressions_equal_p (vro1->op1, vro2->op1)
>      && expressions_equal_p (vro1->op2, vro2->op2);
> --- 387,393 ----
>    const_vn_reference_op_t const vro1 = (const_vn_reference_op_t) p1;
>    const_vn_reference_op_t const vro2 = (const_vn_reference_op_t) p2;
>    return vro1->opcode == vro2->opcode
> !     && types_compatible_p (vro1->type, vro2->type)
>      && expressions_equal_p (vro1->op0, vro2->op0)
>      && expressions_equal_p (vro1->op1, vro2->op1)
>      && expressions_equal_p (vro1->op2, vro2->op2);
> *************** vn_nary_op_eq (const void *p1, const voi
> *** 1184,1190 ****
>    unsigned i;
>
>    if (vno1->opcode != vno2->opcode
> !       || vno1->type != vno2->type)
>      return false;
>
>    for (i = 0; i < vno1->length; ++i)
> --- 1184,1190 ----
>    unsigned i;
>
>    if (vno1->opcode != vno2->opcode
> !       || !types_compatible_p (vno1->type, vno2->type))
>      return false;
>
>    for (i = 0; i < vno1->length; ++i)
> Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-21.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-21.c  (revision 0)
> --- gcc/testsuite/gcc.dg/tree-ssa/ssa-pre-21.c  (revision 0)
> ***************
> *** 0 ****
> --- 1,15 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -fdump-tree-pre" } */
> +
> + long
> + NumSift (long *array, unsigned long k)
> + {
> +   if (array[k] < array[k + 1L])
> +     ++k;
> +   return array[k];
> + }
> +
> + /* There should be only two loads left.  */
> +
> + /* { dg-final { scan-tree-dump-times "= \\\*D" 2 "pre" } } */
> + /* { dg-final { cleanup-tree-dump "pre" } } */
>