This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fold comparison of local addresses


Hi Richard,

It may have caused

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35716


H.J.
On Thu, Mar 27, 2008 at 2:15 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Wed, 26 Mar 2008, Richard Guenther wrote:
>
>  > On Wed, Mar 26, 2008 at 3:10 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>  > > Hi,
>  > >
>  > >  This patch may be the cause for
>  > >
>  > >  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35706
>  > >
>  > >  I will know for sure within an hour.
>  >
>  > Bah, I have bootstrapped and tested a clean tree.  I'll revert the
>  > patch for now.
>
>  Third try.  This time with proper testing on x86_64-unknown-linux-gnu.
>
>  Apart from a typo the patch triggered a latent bug in VRP where that
>  was assuming that fold_binary_to_constant will either return NULL
>  or an integer constant.  Which is not true as that function can
>  return anything that is TREE_CONSTANT.  Whether "foo" < "bar" should
>  really be TREE_CONSTANT is another question though, but rather than
>  trying to reason about that I made VRP more robust here (VRP is
>  the only caller of fold_binary_to_constant with tcc_comparison
>  operators).
>
>  Thus, installed as follows.
>
>
>  Richard.
>
>  2008-03-26  Richard Guenther  <rguenther@suse.de>
>
>         * fold-const.c (target.h): Include.
>         (fold_comparison): Fold comparison of addresses of decls
>         that bind locally or of constants.  Consolidate address folding code.
>         * tree-vrp.c (operand_less_p): Deal with non-INTEGER_CST
>         results from fold_binary_to_constant.
>         (compare_values_warnv): Likewise.
>
>
>         * gcc.dg/fold-addr-1.c: New testcase.
>
>  Index: testsuite/gcc.dg/fold-addr-1.c
>  ===================================================================
>  *** testsuite/gcc.dg/fold-addr-1.c      (revision 0)
>  --- testsuite/gcc.dg/fold-addr-1.c      (revision 0)
>  ***************
>  *** 0 ****
>  --- 1,10 ----
>  + /* { dg-do compile } */
>  + /* { dg-options "-fdump-tree-original" } */
>  +
>  + int bar(char p1, char p2)
>  + {
>  +   return &p1 == &p2;
>  + }
>  +
>  + /* { dg-final { scan-tree-dump "return 0;" "original" } } */
>  + /* { dg-final { cleanup-tree-dump "original" } } */
>  Index: tree-vrp.c
>  ===================================================================
>  *** tree-vrp.c  (revision 133613)
>  --- tree-vrp.c  (working copy)
>  *************** operand_less_p (tree val, tree val2)
>  *** 724,730 ****
>
>         fold_undefer_and_ignore_overflow_warnings ();
>
>  !       if (!tcmp)
>         return -2;
>
>         if (!integer_zerop (tcmp))
>  --- 724,731 ----
>
>         fold_undefer_and_ignore_overflow_warnings ();
>
>  !       if (!tcmp
>  !         || TREE_CODE (tcmp) != INTEGER_CST)
>         return -2;
>
>         if (!integer_zerop (tcmp))
>  *************** compare_values_warnv (tree val1, tree va
>  *** 932,938 ****
>           || TREE_CODE (val2) != INTEGER_CST)
>         {
>             t = fold_binary_to_constant (NE_EXPR, boolean_type_node, val1, val2);
>  !         if (t && tree_expr_nonzero_p (t))
>             return 2;
>         }
>
>  --- 933,939 ----
>           || TREE_CODE (val2) != INTEGER_CST)
>         {
>             t = fold_binary_to_constant (NE_EXPR, boolean_type_node, val1, val2);
>  !         if (t && integer_onep (t))
>             return 2;
>         }
>
>  Index: fold-const.c
>  ===================================================================
>  *** fold-const.c        (revision 133613)
>
>
> --- fold-const.c        (working copy)
>  *************** along with GCC; see the file COPYING3.
>  *** 58,63 ****
>  --- 58,64 ----
>   #include "rtl.h"
>   #include "expr.h"
>   #include "tm_p.h"
>  + #include "target.h"
>   #include "toplev.h"
>   #include "intl.h"
>   #include "ggc.h"
>  *************** fold_comparison (enum tree_code code, tr
>  *** 8481,8491 ****
>         HOST_WIDE_INT bitsize, bitpos0 = 0, bitpos1 = 0;
>         enum machine_mode mode;
>         int volatilep, unsignedp;
>  !       bool indirect_base0 = false;
>
>         /* Get base and offset for the access.  Strip ADDR_EXPR for
>          get_inner_reference, but put it back by stripping INDIRECT_REF
>  !        off the base object if possible.  */
>         base0 = arg0;
>         if (TREE_CODE (arg0) == ADDR_EXPR)
>         {
>  --- 8482,8493 ----
>         HOST_WIDE_INT bitsize, bitpos0 = 0, bitpos1 = 0;
>         enum machine_mode mode;
>         int volatilep, unsignedp;
>  !       bool indirect_base0 = false, indirect_base1 = false;
>
>         /* Get base and offset for the access.  Strip ADDR_EXPR for
>          get_inner_reference, but put it back by stripping INDIRECT_REF
>  !        off the base object if possible.  indirect_baseN will be true
>  !        if baseN is not an address but refers to the object itself.  */
>         base0 = arg0;
>         if (TREE_CODE (arg0) == ADDR_EXPR)
>         {
>  *************** fold_comparison (enum tree_code code, tr
>  *** 8509,8532 ****
>           base1 = get_inner_reference (TREE_OPERAND (arg1, 0),
>                                        &bitsize, &bitpos1, &offset1, &mode,
>                                        &unsignedp, &volatilep, false);
>  !         /* We have to make sure to have an indirect/non-indirect base1
>  !            just the same as we did for base0.  */
>  !         if (TREE_CODE (base1) == INDIRECT_REF
>  !             && !indirect_base0)
>             base1 = TREE_OPERAND (base1, 0);
>  !         else if (!indirect_base0)
>  !           base1 = NULL_TREE;
>         }
>         else if (TREE_CODE (arg1) == POINTER_PLUS_EXPR)
>         {
>           base1 = TREE_OPERAND (arg1, 0);
>           offset1 = TREE_OPERAND (arg1, 1);
>         }
>  -       else if (indirect_base0)
>  -       base1 = NULL_TREE;
>
>         /* If we have equivalent bases we might be able to simplify.  */
>  !       if (base0 && base1
>           && operand_equal_p (base0, base1, 0))
>         {
>           /* We can fold this expression to a constant if the non-constant
>  --- 8511,8529 ----
>           base1 = get_inner_reference (TREE_OPERAND (arg1, 0),
>                                        &bitsize, &bitpos1, &offset1, &mode,
>                                        &unsignedp, &volatilep, false);
>  !         if (TREE_CODE (base1) == INDIRECT_REF)
>             base1 = TREE_OPERAND (base1, 0);
>  !         else
>  !           indirect_base1 = true;
>         }
>         else if (TREE_CODE (arg1) == POINTER_PLUS_EXPR)
>         {
>           base1 = TREE_OPERAND (arg1, 0);
>           offset1 = TREE_OPERAND (arg1, 1);
>         }
>
>         /* If we have equivalent bases we might be able to simplify.  */
>  !       if (indirect_base0 == indirect_base1
>           && operand_equal_p (base0, base1, 0))
>         {
>           /* We can fold this expression to a constant if the non-constant
>  *************** fold_comparison (enum tree_code code, tr
>  *** 8581,8586 ****
>  --- 8578,8621 ----
>
>               return fold_build2 (code, type, offset0, offset1);
>             }
>         }
>  +       /* For non-equal bases we can simplify if they are addresses
>  +        of local binding decls or constants.  */
>
> +       else if (indirect_base0 && indirect_base1
>  +              /* We know that !operand_equal_p (base0, base1, 0)
>  +                 because the if condition was false.  */
>
> +              && TREE_CODE (arg0) == ADDR_EXPR
>  +              && TREE_CODE (arg1) == ADDR_EXPR
>  +              && ((DECL_P (base0)
>  +                   && (targetm.binds_local_p (base0)
>  +                       || CONSTANT_CLASS_P (base1)))
>  +                  || CONSTANT_CLASS_P (base0))
>  +              && ((DECL_P (base1)
>  +                   && (targetm.binds_local_p (base1)
>  +                       || CONSTANT_CLASS_P (base0)))
>  +                  || CONSTANT_CLASS_P (base1)))
>
>
> +       {
>  +         if (code == EQ_EXPR)
>  +           return omit_two_operands (type, boolean_false_node, arg0, arg1);
>  +         else if (code == NE_EXPR)
>  +           return omit_two_operands (type, boolean_true_node, arg0, arg1);
>  +       }
>  +       /* For equal offsets we can simplify to a comparison of the
>  +        base addresses.  */
>  +       else if (bitpos0 == bitpos1
>  +              && (indirect_base0
>  +                  ? base0 != TREE_OPERAND (arg0, 0) : base0 != arg0)
>  +              && (indirect_base1
>  +                  ? base1 != TREE_OPERAND (arg1, 0) : base1 != arg1)
>  +              && ((offset0 == offset1)
>  +                  || (offset0 && offset1
>  +                      && operand_equal_p (offset0, offset1, 0))))
>  +       {
>  +         if (indirect_base0)
>  +           base0 = fold_addr_expr (base0);
>  +         if (indirect_base1)
>  +           base1 = fold_addr_expr (base1);
>  +         return fold_build2 (code, type, base0, base1);
>  +       }
>       }
>
>     /* Transform comparisons of the form X +- C1 CMP Y +- C2 to
>  *************** fold_comparison (enum tree_code code, tr
>  *** 8927,8953 ****
>         }
>       }
>
>  -   /* Fold a comparison of the address of COMPONENT_REFs with the same
>  -      type and component to a comparison of the address of the base
>  -      object.  In short, &x->a OP &y->a to x OP y and
>  -      &x->a OP &y.a to x OP &y  */
>  -   if (TREE_CODE (arg0) == ADDR_EXPR
>  -       && TREE_CODE (TREE_OPERAND (arg0, 0)) == COMPONENT_REF
>  -       && TREE_CODE (arg1) == ADDR_EXPR
>  -       && TREE_CODE (TREE_OPERAND (arg1, 0)) == COMPONENT_REF)
>  -     {
>  -       tree cref0 = TREE_OPERAND (arg0, 0);
>  -       tree cref1 = TREE_OPERAND (arg1, 0);
>  -       if (TREE_OPERAND (cref0, 1) == TREE_OPERAND (cref1, 1))
>  -       {
>  -         tree op0 = TREE_OPERAND (cref0, 0);
>  -         tree op1 = TREE_OPERAND (cref1, 0);
>  -         return fold_build2 (code, type,
>  -                             fold_addr_expr (op0),
>  -                             fold_addr_expr (op1));
>  -       }
>  -     }
>  -
>     /* We can fold X/C1 op C2 where C1 and C2 are integer constants
>        into a single range test.  */
>     if ((TREE_CODE (arg0) == TRUNC_DIV_EXPR
>  --- 8962,8967 ----
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]