Fix folding of EQ/NE_EXPR WRT symtab aliases

Hi,
this patch fixes fold-const folding if EQ/NE_EXPR of ADDR_EXPRS WRT aliases and
weaks. Similarly as my earlier nonzero_address_p patch, it moves the logic
whether two symbls can resolve to same location to symtab.

The existing code did not make much sense to me, so I tried to follow what
alias oracle does - i.e. if we know both bases are decls, but offsets are
different, we know the addresses are different.
If we have offsets same (or we are unsure) we can see if the bases are known
to be same or different.

One important case that the patch fixes is folding &alias == &alias_target as
false.  This avoids late optimization from removing many of the speculatively
inlined functions.A

For virtual function I play even bit unsafe and make alias == &alias_target
even if alias_target is interposable but can not change semantically.
This helps to turn speculative inlining into nonspeculative late in compilation
and should be safe, becuase virtual functions are not compared for addresses in
other contexts.

Bootstrapped/regtested x86_64-linux, seems sane?

Honza

	* c-family/c-common.c: Refuse weak on visibility change.
	* cgraph.h: (sybtam_node::equal_address_to): Declare.
	* fold-const.c (fold_comparison): Use equal_address_to.
	(fold_binary_loc): Likewise.
	* symtab.c (symtab_node::equal_address_to): New predicate.

	* gcc.dg/addr_equal-1.c: New testcase.

On Thu, Dec 4, 2014 at 12:28 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> this patch fixes fold-const folding if EQ/NE_EXPR of ADDR_EXPRS WRT aliases and
> weaks. Similarly as my earlier nonzero_address_p patch, it moves the logic
> whether two symbls can resolve to same location to symtab.
>
> The existing code did not make much sense to me, so I tried to follow what
> alias oracle does - i.e. if we know both bases are decls, but offsets are
> different, we know the addresses are different.
> If we have offsets same (or we are unsure) we can see if the bases are known
> to be same or different.
>
> One important case that the patch fixes is folding &alias == &alias_target as
> false.  This avoids late optimization from removing many of the speculatively
> inlined functions.A
>
> For virtual function I play even bit unsafe and make alias == &alias_target
> even if alias_target is interposable but can not change semantically.
> This helps to turn speculative inlining into nonspeculative late in compilation
> and should be safe, becuase virtual functions are not compared for addresses in
> other contexts.
>
> Bootstrapped/regtested x86_64-linux, seems sane?
>
> Honza
>
>         * c-family/c-common.c: Refuse weak on visibility change.
>         * cgraph.h: (sybtam_node::equal_address_to): Declare.
>         * fold-const.c (fold_comparison): Use equal_address_to.
>         (fold_binary_loc): Likewise.
>         * symtab.c (symtab_node::equal_address_to): New predicate.
>
>         * gcc.dg/addr_equal-1.c: New testcase.
> Index: c-family/c-common.c
> ===================================================================
> --- c-family/c-common.c (revision 218286)
> +++ c-family/c-common.c (working copy)
> @@ -7781,7 +7781,12 @@
>      }
>    else if (TREE_CODE (*node) == FUNCTION_DECL
>            || TREE_CODE (*node) == VAR_DECL)
> -    declare_weak (*node);
> +    {
> +      struct symtab_node *n = symtab_node::get (*node);
> +      if (n && n->refuse_visibility_changes)
> +       error ("%+D declared weak after being used", *node);
> +      declare_weak (*node);
> +    }
>    else
>      warning (OPT_Wattributes, "%qE attribute ignored", name);
>
> Index: cgraph.h
> ===================================================================
> --- cgraph.h    (revision 218286)
> +++ cgraph.h    (working copy)
> @@ -332,6 +332,11 @@
>    /* Return true if symbol is known to be nonzero.  */
>    bool nonzero_address ();
>
> +  /* Return 0 if symbol is known to have different address than S2,
> +     Return 1 if symbol is known to have same address as S2,
> +     return 2 otherwise.   */
> +  int equal_address_to (symtab_node *s2);
> +
>    /* Return symbol table node associated with DECL, if any,
>       and NULL otherwise.  */
>    static inline symtab_node *get (const_tree decl)
> Index: fold-const.c
> ===================================================================
> --- fold-const.c        (revision 218286)
> +++ fold-const.c        (working copy)
> @@ -8980,7 +8981,7 @@
>             }
>         }
>        /* For non-equal bases we can simplify if they are addresses
> -        of local binding decls or constants.  */
> +        declarations with different addresses.  */
>        else if (indirect_base0 && indirect_base1
>                /* We know that !operand_equal_p (base0, base1, 0)
>                   because the if condition was false.  But make
> @@ -8988,16 +8989,13 @@
>                && base0 != base1
>                && TREE_CODE (arg0) == ADDR_EXPR
>                && TREE_CODE (arg1) == ADDR_EXPR
> -              && (((TREE_CODE (base0) == VAR_DECL
> -                    || TREE_CODE (base0) == PARM_DECL)
> -                   && (targetm.binds_local_p (base0)
> -                       || CONSTANT_CLASS_P (base1)))
> -                  || CONSTANT_CLASS_P (base0))
> -              && (((TREE_CODE (base1) == VAR_DECL
> -                    || TREE_CODE (base1) == PARM_DECL)
> -                   && (targetm.binds_local_p (base1)
> -                       || CONSTANT_CLASS_P (base0)))
> -                  || CONSTANT_CLASS_P (base1)))
> +              && DECL_P (base0)
> +              && DECL_P (base1)
> +              /* Watch for aliases.  */
> +              && (!decl_in_symtab_p (base0)
> +                  || !decl_in_symtab_p (base1)
> +                  || !symtab_node::get_create (base0)->equal_address_to
> +                        (symtab_node::get_create (base1))))
>         {
>           if (code == EQ_EXPR)
>             return omit_two_operands_loc (loc, type, boolean_false_node,
> @@ -12248,39 +12246,6 @@
>           && code == NE_EXPR)
>          return non_lvalue_loc (loc, fold_convert_loc (loc, type, arg0));
>
> -      /* If this is an equality comparison of the address of two non-weak,
> -        unaliased symbols neither of which are extern (since we do not
> -        have access to attributes for externs), then we know the result.  */
> -      if (TREE_CODE (arg0) == ADDR_EXPR
> -         && VAR_OR_FUNCTION_DECL_P (TREE_OPERAND (arg0, 0))
> -         && ! DECL_WEAK (TREE_OPERAND (arg0, 0))
> -         && ! lookup_attribute ("alias",
> -                                DECL_ATTRIBUTES (TREE_OPERAND (arg0, 0)))
> -         && ! DECL_EXTERNAL (TREE_OPERAND (arg0, 0))
> -         && TREE_CODE (arg1) == ADDR_EXPR
> -         && VAR_OR_FUNCTION_DECL_P (TREE_OPERAND (arg1, 0))
> -         && ! DECL_WEAK (TREE_OPERAND (arg1, 0))
> -         && ! lookup_attribute ("alias",
> -                                DECL_ATTRIBUTES (TREE_OPERAND (arg1, 0)))
> -         && ! DECL_EXTERNAL (TREE_OPERAND (arg1, 0)))
> -       {
> -         /* We know that we're looking at the address of two
> -            non-weak, unaliased, static _DECL nodes.
> -
> -            It is both wasteful and incorrect to call operand_equal_p
> -            to compare the two ADDR_EXPR nodes.  It is wasteful in that
> -            all we need to do is test pointer equality for the arguments
> -            to the two ADDR_EXPR nodes.  It is incorrect to use
> -            operand_equal_p as that function is NOT equivalent to a
> -            C equality test.  It can in fact return false for two
> -            objects which would test as equal using the C equality
> -            operator.  */
> -         bool equal = TREE_OPERAND (arg0, 0) == TREE_OPERAND (arg1, 0);
> -         return constant_boolean_node (equal
> -                                       ? code == EQ_EXPR : code != EQ_EXPR,
> -                                       type);
> -       }
> -
>        /* Similarly for a NEGATE_EXPR.  */
>        if (TREE_CODE (arg0) == NEGATE_EXPR
>           && TREE_CODE (arg1) == INTEGER_CST
> @@ -12301,6 +12266,104 @@
>                                                            arg1),
>                                          TREE_OPERAND (arg0, 1)));
>
> +      /* Compare two addresses.  */
> +      if (TREE_CODE (arg0) == ADDR_EXPR
> +         && TREE_CODE (arg1) == ADDR_EXPR)
> +       {
> +          HOST_WIDE_INT size0, hwi_offset0, max_size0;
> +          HOST_WIDE_INT size1, hwi_offset1, max_size1;
> +         tree base0 = get_ref_base_and_extent (TREE_OPERAND (arg0, 0), &hwi_offset0, &size0, &max_size0);
> +         tree base1 = get_ref_base_and_extent (TREE_OPERAND (arg1, 0), &hwi_offset1, &size1, &max_size1);

I think you want get_addr_base_and_unit_offset here.  But I really wonder
what cases this code catches that the code in fold_comparison you touched
above does not?  (apart from previously being bogus in different kind of
ways)

So I'd rather remove the code in fold_binary.

Thanks,
Richard.

> +         offset_int offset0 = hwi_offset0;
> +         offset_int offset1 = hwi_offset1;
> +
> +         /* Add constant offset of MEM_REF to OFFSET, if possible.  */
> +         if (base0 && TREE_CODE (base0) == MEM_REF
> +             && TREE_CODE (TREE_OPERAND (base0, 1)) == INTEGER_CST)
> +           {
> +             offset0 += wi::lshift (mem_ref_offset (base0), LOG2_BITS_PER_UNIT);
> +             base0 = TREE_OPERAND (base0, 0);
> +           }
> +         if (base1 && TREE_CODE (base1) == MEM_REF
> +             && TREE_CODE (TREE_OPERAND (base1, 1)) == INTEGER_CST)
> +           {
> +             offset1 += wi::lshift (mem_ref_offset (base1), LOG2_BITS_PER_UNIT);
> +             base1 = TREE_OPERAND (base1, 0);
> +           }
> +         /* If both offsets of MEM_REF are the same, just ignore them.  */
> +         if (base0 && base1
> +             && TREE_CODE (base0) == MEM_REF
> +             && TREE_CODE (base1) == MEM_REF
> +             && operand_equal_p (TREE_OPERAND (base0, 1), TREE_OPERAND (base1, 1), 0))
> +           {
> +             base0 = TREE_OPERAND (base0, 0);
> +             base1 = TREE_OPERAND (base1, 1);
> +           }
> +         /* If we see MEM_REF with variable offset, just modify MAX_SIZE to declare that
> +            sizes are unknown.  We can still prove that bases points to different memory
> +            locations.  */
> +         if (base0 && TREE_CODE (base0) == MEM_REF)
> +           {
> +             base0 = TREE_OPERAND (base0, 0);
> +             max_size0 = -1;
> +           }
> +         if (base1 && TREE_CODE (base1) == MEM_REF)
> +           {
> +             base1 = TREE_OPERAND (base1, 0);
> +             max_size1 = -1;
> +           }
> +
> +         /* If bases are equal or they are both declarations, we can disprove equivalency by
> +            proving that offsets are different.  */
> +         if (base0 && base1 && (base0 == base1 || (DECL_P (base0) && DECL_P (base1))))
> +           {
> +             if ((wi::ltu_p (offset0 + max_size0 - size0, offset1)
> +                  || (wi::ltu_p (offset1 + max_size1 - size1, offset0)))
> +                 && max_size0 != -1 && max_size1 != -1
> +                 && size0 != -1 && size1 != -1)
> +               return constant_boolean_node (code != EQ_EXPR, type);
> +           }
> +         /* If bases are equal, then addresses are equal if offsets are.
> +            We can work hader here for non-constant offsets.  */
> +         if (base0 && base0 == base1)
> +           {
> +             if (base0 == base1
> +                 && size0 != -1 && size1 != -1
> +                 && (max_size0 == size0) && (max_size1 == size1))
> +               return constant_boolean_node (code == EQ_EXPR, type);
> +           }
> +
> +         if (base0 && base1 && DECL_P (base0) && DECL_P (base1))
> +           {
> +             bool in_symtab0 = decl_in_symtab_p (base0);
> +             bool in_symtab1 = decl_in_symtab_p (base1);
> +
> +             /* Symtab and non-symtab declarations never overlap.  */
> +             if (in_symtab0 != in_symtab1)
> +               return constant_boolean_node (code != EQ_EXPR, type);
> +             /* Non-symtab nodes never have aliases: different declaration means
> +                different memory object.  */
> +             if (!in_symtab0)
> +               {
> +                 if (base0 != base1 && TREE_CODE (base0) != TREE_CODE (base1))
> +                   return constant_boolean_node (code != EQ_EXPR, type);
> +               }
> +             else
> +               {
> +                 struct symtab_node *symbol0 = symtab_node::get_create (base0);
> +                 struct symtab_node *symbol1 = symtab_node::get_create (base1);
> +                 int cmp = symbol0->equal_address_to (symbol1);
> +
> +                 if (cmp == 0)
> +                   return constant_boolean_node (code != EQ_EXPR, type);
> +                 if (cmp == 1
> +                     && size0 != -1 && size1 != -1
> +                     && (max_size0 == size0) && (max_size1 == size1))
> +                   return constant_boolean_node (code == EQ_EXPR, type);
> +               }
> +           }
> +       }
> +
>        /* Transform comparisons of the form X +- Y CMP X to Y CMP 0.  */
>        if ((TREE_CODE (arg0) == PLUS_EXPR
>            || TREE_CODE (arg0) == POINTER_PLUS_EXPR
> Index: symtab.c
> ===================================================================
> --- symtab.c    (revision 218286)
> +++ symtab.c    (working copy)
> @@ -1860,3 +1860,90 @@
>      return true;
>    return false;
>  }
> +
> +/* Return 0 if symbol is known to have different address than S2,
> +   Return 1 if symbol is known to have same address as S2,
> +   return 2 otherwise.   */
> +int
> +symtab_node::equal_address_to (symtab_node *s2)
> +{
> +  enum availability avail1, avail2;
> +
> +  /* A Shortcut: equivalent symbols are always equivalent.  */
> +  if (this == s2)
> +    return 1;
> +
> +  /* For non-interposable aliases, lookup and compare their actual definitions.
> +     Also check if the symbol needs to bind to given definition.  */
> +  symtab_node *rs1 = ultimate_alias_target (&avail1);
> +  symtab_node *rs2 = s2->ultimate_alias_target (&avail2);
> +  bool binds_local1 = rs1->analyzed && decl_binds_to_current_def_p (this->decl);
> +  bool binds_local2 = rs2->analyzed && decl_binds_to_current_def_p (s2->decl);
> +  bool really_binds_local1 = binds_local1;
> +  bool really_binds_local2 = binds_local2;
> +
> +  /* Addresses of vtables and virtual functions can not be used by user
> +     code and are used only within speculation.  In this case we may make
> +     symbol equivalent to its alias even if interposition may break this
> +     rule.  Doing so will allow us to turn speculative inlining into
> +     non-speculative more agressively.  */
> +  if (DECL_VIRTUAL_P (this->decl) && avail1 >= AVAIL_AVAILABLE)
> +    binds_local1 = true;
> +  if (DECL_VIRTUAL_P (s2->decl) && avail2 >= AVAIL_AVAILABLE)
> +    binds_local2 = true;
> +
> +  /* If both definitions are available we know that even if they are bound
> +     to other unit they must be defined same way and therefore we can use
> +     equivalence test.  */
> +  if (rs1 != rs2 && avail1 >= AVAIL_AVAILABLE && avail2 >= AVAIL_AVAILABLE)
> +    binds_local1 = binds_local2 = true;
> +
> +  if ((binds_local1 ? rs1 : this)
> +       == (binds_local2 ? rs2 : s2))
> +    {
> +      /* We made use of the fact that alias is not weak.  */
> +      if (binds_local1 && rs1 != this)
> +        refuse_visibility_changes = true;
> +      if (binds_local2 && rs2 != s2)
> +        s2->refuse_visibility_changes = true;
> +      return 1;
> +    }
> +
> +  /* If both symbols may resolve to NULL, we can not really prove them different.  */
> +  if (!nonzero_address () && !s2->nonzero_address ())
> +    return 2;
> +
> +  /* Except for NULL, functions and variables never overlap.  */
> +  if (TREE_CODE (decl) != TREE_CODE (s2->decl))
> +    return 0;
> +
> +  /* If one of the symbols is unresolved alias, punt.  */
> +  if (rs1->alias || rs2->alias)
> +    return 2;
> +
> +  /* If we have a non-interposale definition of at least one of the symbols
> +     and the other symbol is different, we know other unit can not interpose
> +     it to the first symbol; all aliases of the definition needs to be
> +     present in the current unit.  */
> +  if (((really_binds_local1 || really_binds_local2)
> +      /* If we have both definitions and they are different, we know they
> +        will be different even in units they binds to.  */
> +       || (binds_local1 && binds_local2))
> +      && rs1 != rs2)
> +    {
> +      /* We make use of the fact that one symbol is not alias of the other
> +        and that the definition is non-interposable.  */
> +      refuse_visibility_changes = true;
> +      s2->refuse_visibility_changes = true;
> +      rs1->refuse_visibility_changes = true;
> +      rs2->refuse_visibility_changes = true;
> +      return 0;
> +    }
> +
> +  /* TODO: Alias oracle basically assume that addresses of global variables
> +     are different unless they are declared as alias of one to another.
> +     We probably should be consistent and use this fact here, too, and update
> +     alias oracle to use this predicate.  */
> +
> +  return 2;
> +}
> Index: testsuite/gcc.dg/addr_equal-1.c
> ===================================================================
> --- testsuite/gcc.dg/addr_equal-1.c     (revision 0)
> +++ testsuite/gcc.dg/addr_equal-1.c     (working copy)
> @@ -0,0 +1,107 @@
> +/* { dg-do run } */
> +/* { dg-require-weak "" } */
> +/* { dg-require-alias "" } */
> +/* { dg-options "-O2" } */
> +void abort (void);
> +extern int undef_var0, undef_var1;
> +extern __attribute__ ((weak)) int weak_undef_var0;
> +extern __attribute__ ((weak)) int weak_undef_var1;
> +__attribute__ ((weak)) int weak_def_var0;
> +int def_var0=0, def_var1=0;
> +static int alias_var0 __attribute__ ((alias("def_var0")));
> +extern int weak_alias_var0 __attribute__ ((alias("def_var0"))) __attribute__ ((weak));
> +void undef_fn0(void);
> +void undef_fn1(void);
> +void def_fn0(void)
> +{
> +}
> +void def_fn1(void)
> +{
> +}
> +__attribute__ ((weak))
> +void weak_def_fn0(void)
> +{
> +}
> +__attribute__ ((weak))
> +void weak_def_fn1(void)
> +{
> +}
> +__attribute__ ((weak)) void weak_undef_fn0(void);
> +
> +inline
> +void inline_fn0(void)
> +{
> +}
> +inline
> +void inline_fn1(void)
> +{
> +}
> +
> +int
> +main(int argc, char **argv)
> +{
> +  /* Two definitions are always different unless they can be interposed.  */
> +  if (!__builtin_constant_p (def_fn0 == def_fn1))
> +    abort();
> +  if (def_fn0 == def_fn1)
> +    abort();
> +
> +  if (!__builtin_constant_p (&def_var0 == &def_var1))
> +    abort();
> +  if (&def_var0 == &def_var1)
> +    abort();
> +
> +  /* Same symbol is the same no matter on interposition.  */
> +  if (!__builtin_constant_p (undef_fn0 != undef_fn0))
> +    abort ();
> +  if (undef_fn0 != undef_fn0)
> +    abort ();
> +
> +  /* Do not get confused by same offset.  */
> +  if (!__builtin_constant_p (&undef_var0 + argc != &undef_var0 + argc))
> +    abort ();
> +  if (&undef_var0 + argc != &undef_var0 + argc)
> +    abort ();
> +
> +  /* Alias and its target is equivalent unless one of them can be interposed.  */
> +  if (!__builtin_constant_p (&def_var0 != &alias_var0))
> +    abort ();
> +  if (&def_var0 != &alias_var0 )
> +    abort ();
> +
> +  if (__builtin_constant_p (&def_var0 != &weak_alias_var0))
> +    abort ();
> +  if (&def_var0 != &weak_alias_var0)
> +    abort ();
> +
> +  /* Weak definitions may be both NULL.  */
> +  if (__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_undef_var0))
> +    abort ();
> +  if ((void *)weak_undef_fn0 != (void *)&weak_undef_var0)
> +    abort ();
> +
> +  /* Different offsets makes it safe to assume addresses are different.  */
> +  if (!__builtin_constant_p ((char *)weak_undef_fn0 + 4 != (char *)&weak_undef_var1 + 8))
> +    abort ();
> +  if ((char *)weak_undef_fn0 + 4 == (char *)&weak_undef_var1 + 8)
> +    abort ();
> +
> +  /* Variables and functions do not share same memory locations otherwise.  */
> +  if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0))
> +    abort ();
> +  if ((void *)undef_fn0 == (void *)&undef_var0)
> +    abort ();
> +
> +  /* This works for cases where one object is just weakly defined, too.  */
> +  if (!__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_def_var0))
> +    abort ();
> +  if ((void *)weak_undef_fn0 == (void *)&weak_def_var0)
> +    abort ();
> +
> +  /* Inline functions are known to be different.  */
> +  if (!__builtin_constant_p (inline_fn0 != inline_fn1))
> +    abort ();
> +  if (inline_fn0 == inline_fn1)
> +    abort ();
> +  return 0;
> +}

> 
> I think you want get_addr_base_and_unit_offset here.  But I really wonder

I copied what I found in tree-ssa-alias. The differenc eis that
get_addr_base_and_unit_offset won't give a range for variable sized accesses right?

> what cases this code catches that the code in fold_comparison you touched
> above does not?  (apart from previously being bogus in different kind of
> ways)
> 
> So I'd rather remove the code in fold_binary.

Well, the code in fold_binary only handles comparsions where base addresses are known
to be different. I.e. &a==&b returning false.
All the other cases are handled here.  I.. &a==&a or &a[5]==&b[7] etc.
Perhaps the code should be in fold_comparison?

Honza
> 
> Thanks,
> Richard.
> 
> > +         offset_int offset0 = hwi_offset0;
> > +         offset_int offset1 = hwi_offset1;
> > +
> > +         /* Add constant offset of MEM_REF to OFFSET, if possible.  */
> > +         if (base0 && TREE_CODE (base0) == MEM_REF
> > +             && TREE_CODE (TREE_OPERAND (base0, 1)) == INTEGER_CST)
> > +           {
> > +             offset0 += wi::lshift (mem_ref_offset (base0), LOG2_BITS_PER_UNIT);
> > +             base0 = TREE_OPERAND (base0, 0);
> > +           }
> > +         if (base1 && TREE_CODE (base1) == MEM_REF
> > +             && TREE_CODE (TREE_OPERAND (base1, 1)) == INTEGER_CST)
> > +           {
> > +             offset1 += wi::lshift (mem_ref_offset (base1), LOG2_BITS_PER_UNIT);
> > +             base1 = TREE_OPERAND (base1, 0);
> > +           }
> > +         /* If both offsets of MEM_REF are the same, just ignore them.  */
> > +         if (base0 && base1
> > +             && TREE_CODE (base0) == MEM_REF
> > +             && TREE_CODE (base1) == MEM_REF
> > +             && operand_equal_p (TREE_OPERAND (base0, 1), TREE_OPERAND (base1, 1), 0))
> > +           {
> > +             base0 = TREE_OPERAND (base0, 0);
> > +             base1 = TREE_OPERAND (base1, 1);
> > +           }
> > +         /* If we see MEM_REF with variable offset, just modify MAX_SIZE to declare that
> > +            sizes are unknown.  We can still prove that bases points to different memory
> > +            locations.  */
> > +         if (base0 && TREE_CODE (base0) == MEM_REF)
> > +           {
> > +             base0 = TREE_OPERAND (base0, 0);
> > +             max_size0 = -1;
> > +           }
> > +         if (base1 && TREE_CODE (base1) == MEM_REF)
> > +           {
> > +             base1 = TREE_OPERAND (base1, 0);
> > +             max_size1 = -1;
> > +           }
> > +
> > +         /* If bases are equal or they are both declarations, we can disprove equivalency by
> > +            proving that offsets are different.  */
> > +         if (base0 && base1 && (base0 == base1 || (DECL_P (base0) && DECL_P (base1))))
> > +           {
> > +             if ((wi::ltu_p (offset0 + max_size0 - size0, offset1)
> > +                  || (wi::ltu_p (offset1 + max_size1 - size1, offset0)))
> > +                 && max_size0 != -1 && max_size1 != -1
> > +                 && size0 != -1 && size1 != -1)
> > +               return constant_boolean_node (code != EQ_EXPR, type);
> > +           }
> > +         /* If bases are equal, then addresses are equal if offsets are.
> > +            We can work hader here for non-constant offsets.  */
> > +         if (base0 && base0 == base1)
> > +           {
> > +             if (base0 == base1
> > +                 && size0 != -1 && size1 != -1
> > +                 && (max_size0 == size0) && (max_size1 == size1))
> > +               return constant_boolean_node (code == EQ_EXPR, type);
> > +           }
> > +
> > +         if (base0 && base1 && DECL_P (base0) && DECL_P (base1))
> > +           {
> > +             bool in_symtab0 = decl_in_symtab_p (base0);
> > +             bool in_symtab1 = decl_in_symtab_p (base1);
> > +
> > +             /* Symtab and non-symtab declarations never overlap.  */
> > +             if (in_symtab0 != in_symtab1)
> > +               return constant_boolean_node (code != EQ_EXPR, type);
> > +             /* Non-symtab nodes never have aliases: different declaration means
> > +                different memory object.  */
> > +             if (!in_symtab0)
> > +               {
> > +                 if (base0 != base1 && TREE_CODE (base0) != TREE_CODE (base1))
> > +                   return constant_boolean_node (code != EQ_EXPR, type);
> > +               }
> > +             else
> > +               {
> > +                 struct symtab_node *symbol0 = symtab_node::get_create (base0);
> > +                 struct symtab_node *symbol1 = symtab_node::get_create (base1);
> > +                 int cmp = symbol0->equal_address_to (symbol1);
> > +
> > +                 if (cmp == 0)
> > +                   return constant_boolean_node (code != EQ_EXPR, type);
> > +                 if (cmp == 1
> > +                     && size0 != -1 && size1 != -1
> > +                     && (max_size0 == size0) && (max_size1 == size1))
> > +                   return constant_boolean_node (code == EQ_EXPR, type);
> > +               }
> > +           }
> > +       }
> > +
> >        /* Transform comparisons of the form X +- Y CMP X to Y CMP 0.  */
> >        if ((TREE_CODE (arg0) == PLUS_EXPR
> >            || TREE_CODE (arg0) == POINTER_PLUS_EXPR
> > Index: symtab.c
> > ===================================================================
> > --- symtab.c    (revision 218286)
> > +++ symtab.c    (working copy)
> > @@ -1860,3 +1860,90 @@
> >      return true;
> >    return false;
> >  }
> > +
> > +/* Return 0 if symbol is known to have different address than S2,
> > +   Return 1 if symbol is known to have same address as S2,
> > +   return 2 otherwise.   */
> > +int
> > +symtab_node::equal_address_to (symtab_node *s2)
> > +{
> > +  enum availability avail1, avail2;
> > +
> > +  /* A Shortcut: equivalent symbols are always equivalent.  */
> > +  if (this == s2)
> > +    return 1;
> > +
> > +  /* For non-interposable aliases, lookup and compare their actual definitions.
> > +     Also check if the symbol needs to bind to given definition.  */
> > +  symtab_node *rs1 = ultimate_alias_target (&avail1);
> > +  symtab_node *rs2 = s2->ultimate_alias_target (&avail2);
> > +  bool binds_local1 = rs1->analyzed && decl_binds_to_current_def_p (this->decl);
> > +  bool binds_local2 = rs2->analyzed && decl_binds_to_current_def_p (s2->decl);
> > +  bool really_binds_local1 = binds_local1;
> > +  bool really_binds_local2 = binds_local2;
> > +
> > +  /* Addresses of vtables and virtual functions can not be used by user
> > +     code and are used only within speculation.  In this case we may make
> > +     symbol equivalent to its alias even if interposition may break this
> > +     rule.  Doing so will allow us to turn speculative inlining into
> > +     non-speculative more agressively.  */
> > +  if (DECL_VIRTUAL_P (this->decl) && avail1 >= AVAIL_AVAILABLE)
> > +    binds_local1 = true;
> > +  if (DECL_VIRTUAL_P (s2->decl) && avail2 >= AVAIL_AVAILABLE)
> > +    binds_local2 = true;
> > +
> > +  /* If both definitions are available we know that even if they are bound
> > +     to other unit they must be defined same way and therefore we can use
> > +     equivalence test.  */
> > +  if (rs1 != rs2 && avail1 >= AVAIL_AVAILABLE && avail2 >= AVAIL_AVAILABLE)
> > +    binds_local1 = binds_local2 = true;
> > +
> > +  if ((binds_local1 ? rs1 : this)
> > +       == (binds_local2 ? rs2 : s2))
> > +    {
> > +      /* We made use of the fact that alias is not weak.  */
> > +      if (binds_local1 && rs1 != this)
> > +        refuse_visibility_changes = true;
> > +      if (binds_local2 && rs2 != s2)
> > +        s2->refuse_visibility_changes = true;
> > +      return 1;
> > +    }
> > +
> > +  /* If both symbols may resolve to NULL, we can not really prove them different.  */
> > +  if (!nonzero_address () && !s2->nonzero_address ())
> > +    return 2;
> > +
> > +  /* Except for NULL, functions and variables never overlap.  */
> > +  if (TREE_CODE (decl) != TREE_CODE (s2->decl))
> > +    return 0;
> > +
> > +  /* If one of the symbols is unresolved alias, punt.  */
> > +  if (rs1->alias || rs2->alias)
> > +    return 2;
> > +
> > +  /* If we have a non-interposale definition of at least one of the symbols
> > +     and the other symbol is different, we know other unit can not interpose
> > +     it to the first symbol; all aliases of the definition needs to be
> > +     present in the current unit.  */
> > +  if (((really_binds_local1 || really_binds_local2)
> > +      /* If we have both definitions and they are different, we know they
> > +        will be different even in units they binds to.  */
> > +       || (binds_local1 && binds_local2))
> > +      && rs1 != rs2)
> > +    {
> > +      /* We make use of the fact that one symbol is not alias of the other
> > +        and that the definition is non-interposable.  */
> > +      refuse_visibility_changes = true;
> > +      s2->refuse_visibility_changes = true;
> > +      rs1->refuse_visibility_changes = true;
> > +      rs2->refuse_visibility_changes = true;
> > +      return 0;
> > +    }
> > +
> > +  /* TODO: Alias oracle basically assume that addresses of global variables
> > +     are different unless they are declared as alias of one to another.
> > +     We probably should be consistent and use this fact here, too, and update
> > +     alias oracle to use this predicate.  */
> > +
> > +  return 2;
> > +}
> > Index: testsuite/gcc.dg/addr_equal-1.c
> > ===================================================================
> > --- testsuite/gcc.dg/addr_equal-1.c     (revision 0)
> > +++ testsuite/gcc.dg/addr_equal-1.c     (working copy)
> > @@ -0,0 +1,107 @@
> > +/* { dg-do run } */
> > +/* { dg-require-weak "" } */
> > +/* { dg-require-alias "" } */
> > +/* { dg-options "-O2" } */
> > +void abort (void);
> > +extern int undef_var0, undef_var1;
> > +extern __attribute__ ((weak)) int weak_undef_var0;
> > +extern __attribute__ ((weak)) int weak_undef_var1;
> > +__attribute__ ((weak)) int weak_def_var0;
> > +int def_var0=0, def_var1=0;
> > +static int alias_var0 __attribute__ ((alias("def_var0")));
> > +extern int weak_alias_var0 __attribute__ ((alias("def_var0"))) __attribute__ ((weak));
> > +void undef_fn0(void);
> > +void undef_fn1(void);
> > +void def_fn0(void)
> > +{
> > +}
> > +void def_fn1(void)
> > +{
> > +}
> > +__attribute__ ((weak))
> > +void weak_def_fn0(void)
> > +{
> > +}
> > +__attribute__ ((weak))
> > +void weak_def_fn1(void)
> > +{
> > +}
> > +__attribute__ ((weak)) void weak_undef_fn0(void);
> > +
> > +inline
> > +void inline_fn0(void)
> > +{
> > +}
> > +inline
> > +void inline_fn1(void)
> > +{
> > +}
> > +
> > +int
> > +main(int argc, char **argv)
> > +{
> > +  /* Two definitions are always different unless they can be interposed.  */
> > +  if (!__builtin_constant_p (def_fn0 == def_fn1))
> > +    abort();
> > +  if (def_fn0 == def_fn1)
> > +    abort();
> > +
> > +  if (!__builtin_constant_p (&def_var0 == &def_var1))
> > +    abort();
> > +  if (&def_var0 == &def_var1)
> > +    abort();
> > +
> > +  /* Same symbol is the same no matter on interposition.  */
> > +  if (!__builtin_constant_p (undef_fn0 != undef_fn0))
> > +    abort ();
> > +  if (undef_fn0 != undef_fn0)
> > +    abort ();
> > +
> > +  /* Do not get confused by same offset.  */
> > +  if (!__builtin_constant_p (&undef_var0 + argc != &undef_var0 + argc))
> > +    abort ();
> > +  if (&undef_var0 + argc != &undef_var0 + argc)
> > +    abort ();
> > +
> > +  /* Alias and its target is equivalent unless one of them can be interposed.  */
> > +  if (!__builtin_constant_p (&def_var0 != &alias_var0))
> > +    abort ();
> > +  if (&def_var0 != &alias_var0 )
> > +    abort ();
> > +
> > +  if (__builtin_constant_p (&def_var0 != &weak_alias_var0))
> > +    abort ();
> > +  if (&def_var0 != &weak_alias_var0)
> > +    abort ();
> > +
> > +  /* Weak definitions may be both NULL.  */
> > +  if (__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_undef_var0))
> > +    abort ();
> > +  if ((void *)weak_undef_fn0 != (void *)&weak_undef_var0)
> > +    abort ();
> > +
> > +  /* Different offsets makes it safe to assume addresses are different.  */
> > +  if (!__builtin_constant_p ((char *)weak_undef_fn0 + 4 != (char *)&weak_undef_var1 + 8))
> > +    abort ();
> > +  if ((char *)weak_undef_fn0 + 4 == (char *)&weak_undef_var1 + 8)
> > +    abort ();
> > +
> > +  /* Variables and functions do not share same memory locations otherwise.  */
> > +  if (!__builtin_constant_p ((void *)undef_fn0 == (void *)&undef_var0))
> > +    abort ();
> > +  if ((void *)undef_fn0 == (void *)&undef_var0)
> > +    abort ();
> > +
> > +  /* This works for cases where one object is just weakly defined, too.  */
> > +  if (!__builtin_constant_p ((void *)weak_undef_fn0 == (void *)&weak_def_var0))
> > +    abort ();
> > +  if ((void *)weak_undef_fn0 == (void *)&weak_def_var0)
> > +    abort ();
> > +
> > +  /* Inline functions are known to be different.  */
> > +  if (!__builtin_constant_p (inline_fn0 != inline_fn1))
> > +    abort ();
> > +  if (inline_fn0 == inline_fn1)
> > +    abort ();
> > +  return 0;
> > +}

On December 4, 2014 6:34:18 PM CET, Jan Hubicka <hubicka@ucw.cz> wrote:
>> 
>> I think you want get_addr_base_and_unit_offset here.  But I really
>wonder
>
>I copied what I found in tree-ssa-alias. The differenc eis that
>get_addr_base_and_unit_offset won't give a range for variable sized
>accesses right?

Yes. But OTOH you could also use non_aliasing_component_refs?

>> what cases this code catches that the code in fold_comparison you
>touched
>> above does not?  (apart from previously being bogus in different kind
>of
>> ways)
>> 
>> So I'd rather remove the code in fold_binary.
>
>Well, the code in fold_binary only handles comparsions where base
>addresses are known
>to be different. I.e. &a==&b returning false.
>All the other cases are handled here.  I.. &a==&a or &a[5]==&b[7] etc.
>Perhaps the code should be in fold_comparison?

Yes, I think it should be in one place.

Richard.

>Honza
>> 
>> Thanks,
>> Richard.
>> 
>> > +         offset_int offset0 = hwi_offset0;
>> > +         offset_int offset1 = hwi_offset1;
>> > +
>> > +         /* Add constant offset of MEM_REF to OFFSET, if possible.
> */
>> > +         if (base0 && TREE_CODE (base0) == MEM_REF
>> > +             && TREE_CODE (TREE_OPERAND (base0, 1)) ==
>INTEGER_CST)
>> > +           {
>> > +             offset0 += wi::lshift (mem_ref_offset (base0),
>LOG2_BITS_PER_UNIT);
>> > +             base0 = TREE_OPERAND (base0, 0);
>> > +           }
>> > +         if (base1 && TREE_CODE (base1) == MEM_REF
>> > +             && TREE_CODE (TREE_OPERAND (base1, 1)) ==
>INTEGER_CST)
>> > +           {
>> > +             offset1 += wi::lshift (mem_ref_offset (base1),
>LOG2_BITS_PER_UNIT);
>> > +             base1 = TREE_OPERAND (base1, 0);
>> > +           }
>> > +         /* If both offsets of MEM_REF are the same, just ignore
>them.  */
>> > +         if (base0 && base1
>> > +             && TREE_CODE (base0) == MEM_REF
>> > +             && TREE_CODE (base1) == MEM_REF
>> > +             && operand_equal_p (TREE_OPERAND (base0, 1),
>TREE_OPERAND (base1, 1), 0))
>> > +           {
>> > +             base0 = TREE_OPERAND (base0, 0);
>> > +             base1 = TREE_OPERAND (base1, 1);
>> > +           }
>> > +         /* If we see MEM_REF with variable offset, just modify
>MAX_SIZE to declare that
>> > +            sizes are unknown.  We can still prove that bases
>points to different memory
>> > +            locations.  */
>> > +         if (base0 && TREE_CODE (base0) == MEM_REF)
>> > +           {
>> > +             base0 = TREE_OPERAND (base0, 0);
>> > +             max_size0 = -1;
>> > +           }
>> > +         if (base1 && TREE_CODE (base1) == MEM_REF)
>> > +           {
>> > +             base1 = TREE_OPERAND (base1, 0);
>> > +             max_size1 = -1;
>> > +           }
>> > +
>> > +         /* If bases are equal or they are both declarations, we
>can disprove equivalency by
>> > +            proving that offsets are different.  */
>> > +         if (base0 && base1 && (base0 == base1 || (DECL_P (base0)
>&& DECL_P (base1))))
>> > +           {
>> > +             if ((wi::ltu_p (offset0 + max_size0 - size0, offset1)
>> > +                  || (wi::ltu_p (offset1 + max_size1 - size1,
>offset0)))
>> > +                 && max_size0 != -1 && max_size1 != -1
>> > +                 && size0 != -1 && size1 != -1)
>> > +               return constant_boolean_node (code != EQ_EXPR,
>type);
>> > +           }
>> > +         /* If bases are equal, then addresses are equal if
>offsets are.
>> > +            We can work hader here for non-constant offsets.  */
>> > +         if (base0 && base0 == base1)
>> > +           {
>> > +             if (base0 == base1
>> > +                 && size0 != -1 && size1 != -1
>> > +                 && (max_size0 == size0) && (max_size1 == size1))
>> > +               return constant_boolean_node (code == EQ_EXPR,
>type);
>> > +           }
>> > +
>> > +         if (base0 && base1 && DECL_P (base0) && DECL_P (base1))
>> > +           {
>> > +             bool in_symtab0 = decl_in_symtab_p (base0);
>> > +             bool in_symtab1 = decl_in_symtab_p (base1);
>> > +
>> > +             /* Symtab and non-symtab declarations never overlap. 
>*/
>> > +             if (in_symtab0 != in_symtab1)
>> > +               return constant_boolean_node (code != EQ_EXPR,
>type);
>> > +             /* Non-symtab nodes never have aliases: different
>declaration means
>> > +                different memory object.  */
>> > +             if (!in_symtab0)
>> > +               {
>> > +                 if (base0 != base1 && TREE_CODE (base0) !=
>TREE_CODE (base1))
>> > +                   return constant_boolean_node (code != EQ_EXPR,
>type);
>> > +               }
>> > +             else
>> > +               {
>> > +                 struct symtab_node *symbol0 =
>symtab_node::get_create (base0);
>> > +                 struct symtab_node *symbol1 =
>symtab_node::get_create (base1);
>> > +                 int cmp = symbol0->equal_address_to (symbol1);
>> > +
>> > +                 if (cmp == 0)
>> > +                   return constant_boolean_node (code != EQ_EXPR,
>type);
>> > +                 if (cmp == 1
>> > +                     && size0 != -1 && size1 != -1
>> > +                     && (max_size0 == size0) && (max_size1 ==
>size1))
>> > +                   return constant_boolean_node (code == EQ_EXPR,
>type);
>> > +               }
>> > +           }
>> > +       }
>> > +
>> >        /* Transform comparisons of the form X +- Y CMP X to Y CMP
>0.  */
>> >        if ((TREE_CODE (arg0) == PLUS_EXPR
>> >            || TREE_CODE (arg0) == POINTER_PLUS_EXPR
>> > Index: symtab.c
>> > ===================================================================
>> > --- symtab.c    (revision 218286)
>> > +++ symtab.c    (working copy)
>> > @@ -1860,3 +1860,90 @@
>> >      return true;
>> >    return false;
>> >  }
>> > +
>> > +/* Return 0 if symbol is known to have different address than S2,
>> > +   Return 1 if symbol is known to have same address as S2,
>> > +   return 2 otherwise.   */
>> > +int
>> > +symtab_node::equal_address_to (symtab_node *s2)
>> > +{
>> > +  enum availability avail1, avail2;
>> > +
>> > +  /* A Shortcut: equivalent symbols are always equivalent.  */
>> > +  if (this == s2)
>> > +    return 1;
>> > +
>> > +  /* For non-interposable aliases, lookup and compare their actual
>definitions.
>> > +     Also check if the symbol needs to bind to given definition. 
>*/
>> > +  symtab_node *rs1 = ultimate_alias_target (&avail1);
>> > +  symtab_node *rs2 = s2->ultimate_alias_target (&avail2);
>> > +  bool binds_local1 = rs1->analyzed && decl_binds_to_current_def_p
>(this->decl);
>> > +  bool binds_local2 = rs2->analyzed && decl_binds_to_current_def_p
>(s2->decl);
>> > +  bool really_binds_local1 = binds_local1;
>> > +  bool really_binds_local2 = binds_local2;
>> > +
>> > +  /* Addresses of vtables and virtual functions can not be used by
>user
>> > +     code and are used only within speculation.  In this case we
>may make
>> > +     symbol equivalent to its alias even if interposition may
>break this
>> > +     rule.  Doing so will allow us to turn speculative inlining
>into
>> > +     non-speculative more agressively.  */
>> > +  if (DECL_VIRTUAL_P (this->decl) && avail1 >= AVAIL_AVAILABLE)
>> > +    binds_local1 = true;
>> > +  if (DECL_VIRTUAL_P (s2->decl) && avail2 >= AVAIL_AVAILABLE)
>> > +    binds_local2 = true;
>> > +
>> > +  /* If both definitions are available we know that even if they
>are bound
>> > +     to other unit they must be defined same way and therefore we
>can use
>> > +     equivalence test.  */
>> > +  if (rs1 != rs2 && avail1 >= AVAIL_AVAILABLE && avail2 >=
>AVAIL_AVAILABLE)
>> > +    binds_local1 = binds_local2 = true;
>> > +
>> > +  if ((binds_local1 ? rs1 : this)
>> > +       == (binds_local2 ? rs2 : s2))
>> > +    {
>> > +      /* We made use of the fact that alias is not weak.  */
>> > +      if (binds_local1 && rs1 != this)
>> > +        refuse_visibility_changes = true;
>> > +      if (binds_local2 && rs2 != s2)
>> > +        s2->refuse_visibility_changes = true;
>> > +      return 1;
>> > +    }
>> > +
>> > +  /* If both symbols may resolve to NULL, we can not really prove
>them different.  */
>> > +  if (!nonzero_address () && !s2->nonzero_address ())
>> > +    return 2;
>> > +
>> > +  /* Except for NULL, functions and variables never overlap.  */
>> > +  if (TREE_CODE (decl) != TREE_CODE (s2->decl))
>> > +    return 0;
>> > +
>> > +  /* If one of the symbols is unresolved alias, punt.  */
>> > +  if (rs1->alias || rs2->alias)
>> > +    return 2;
>> > +
>> > +  /* If we have a non-interposale definition of at least one of
>the symbols
>> > +     and the other symbol is different, we know other unit can not
>interpose
>> > +     it to the first symbol; all aliases of the definition needs
>to be
>> > +     present in the current unit.  */
>> > +  if (((really_binds_local1 || really_binds_local2)
>> > +      /* If we have both definitions and they are different, we
>know they
>> > +        will be different even in units they binds to.  */
>> > +       || (binds_local1 && binds_local2))
>> > +      && rs1 != rs2)
>> > +    {
>> > +      /* We make use of the fact that one symbol is not alias of
>the other
>> > +        and that the definition is non-interposable.  */
>> > +      refuse_visibility_changes = true;
>> > +      s2->refuse_visibility_changes = true;
>> > +      rs1->refuse_visibility_changes = true;
>> > +      rs2->refuse_visibility_changes = true;
>> > +      return 0;
>> > +    }
>> > +
>> > +  /* TODO: Alias oracle basically assume that addresses of global
>variables
>> > +     are different unless they are declared as alias of one to
>another.
>> > +     We probably should be consistent and use this fact here, too,
>and update
>> > +     alias oracle to use this predicate.  */
>> > +
>> > +  return 2;
>> > +}
>> > Index: testsuite/gcc.dg/addr_equal-1.c
>> > ===================================================================
>> > --- testsuite/gcc.dg/addr_equal-1.c     (revision 0)
>> > +++ testsuite/gcc.dg/addr_equal-1.c     (working copy)
>> > @@ -0,0 +1,107 @@
>> > +/* { dg-do run } */
>> > +/* { dg-require-weak "" } */
>> > +/* { dg-require-alias "" } */
>> > +/* { dg-options "-O2" } */
>> > +void abort (void);
>> > +extern int undef_var0, undef_var1;
>> > +extern __attribute__ ((weak)) int weak_undef_var0;
>> > +extern __attribute__ ((weak)) int weak_undef_var1;
>> > +__attribute__ ((weak)) int weak_def_var0;
>> > +int def_var0=0, def_var1=0;
>> > +static int alias_var0 __attribute__ ((alias("def_var0")));
>> > +extern int weak_alias_var0 __attribute__ ((alias("def_var0")))
>__attribute__ ((weak));
>> > +void undef_fn0(void);
>> > +void undef_fn1(void);
>> > +void def_fn0(void)
>> > +{
>> > +}
>> > +void def_fn1(void)
>> > +{
>> > +}
>> > +__attribute__ ((weak))
>> > +void weak_def_fn0(void)
>> > +{
>> > +}
>> > +__attribute__ ((weak))
>> > +void weak_def_fn1(void)
>> > +{
>> > +}
>> > +__attribute__ ((weak)) void weak_undef_fn0(void);
>> > +
>> > +inline
>> > +void inline_fn0(void)
>> > +{
>> > +}
>> > +inline
>> > +void inline_fn1(void)
>> > +{
>> > +}
>> > +
>> > +int
>> > +main(int argc, char **argv)
>> > +{
>> > +  /* Two definitions are always different unless they can be
>interposed.  */
>> > +  if (!__builtin_constant_p (def_fn0 == def_fn1))
>> > +    abort();
>> > +  if (def_fn0 == def_fn1)
>> > +    abort();
>> > +
>> > +  if (!__builtin_constant_p (&def_var0 == &def_var1))
>> > +    abort();
>> > +  if (&def_var0 == &def_var1)
>> > +    abort();
>> > +
>> > +  /* Same symbol is the same no matter on interposition.  */
>> > +  if (!__builtin_constant_p (undef_fn0 != undef_fn0))
>> > +    abort ();
>> > +  if (undef_fn0 != undef_fn0)
>> > +    abort ();
>> > +
>> > +  /* Do not get confused by same offset.  */
>> > +  if (!__builtin_constant_p (&undef_var0 + argc != &undef_var0 +
>argc))
>> > +    abort ();
>> > +  if (&undef_var0 + argc != &undef_var0 + argc)
>> > +    abort ();
>> > +
>> > +  /* Alias and its target is equivalent unless one of them can be
>interposed.  */
>> > +  if (!__builtin_constant_p (&def_var0 != &alias_var0))
>> > +    abort ();
>> > +  if (&def_var0 != &alias_var0 )
>> > +    abort ();
>> > +
>> > +  if (__builtin_constant_p (&def_var0 != &weak_alias_var0))
>> > +    abort ();
>> > +  if (&def_var0 != &weak_alias_var0)
>> > +    abort ();
>> > +
>> > +  /* Weak definitions may be both NULL.  */
>> > +  if (__builtin_constant_p ((void *)weak_undef_fn0 == (void
>*)&weak_undef_var0))
>> > +    abort ();
>> > +  if ((void *)weak_undef_fn0 != (void *)&weak_undef_var0)
>> > +    abort ();
>> > +
>> > +  /* Different offsets makes it safe to assume addresses are
>different.  */
>> > +  if (!__builtin_constant_p ((char *)weak_undef_fn0 + 4 != (char
>*)&weak_undef_var1 + 8))
>> > +    abort ();
>> > +  if ((char *)weak_undef_fn0 + 4 == (char *)&weak_undef_var1 + 8)
>> > +    abort ();
>> > +
>> > +  /* Variables and functions do not share same memory locations
>otherwise.  */
>> > +  if (!__builtin_constant_p ((void *)undef_fn0 == (void
>*)&undef_var0))
>> > +    abort ();
>> > +  if ((void *)undef_fn0 == (void *)&undef_var0)
>> > +    abort ();
>> > +
>> > +  /* This works for cases where one object is just weakly defined,
>too.  */
>> > +  if (!__builtin_constant_p ((void *)weak_undef_fn0 == (void
>*)&weak_def_var0))
>> > +    abort ();
>> > +  if ((void *)weak_undef_fn0 == (void *)&weak_def_var0)
>> > +    abort ();
>> > +
>> > +  /* Inline functions are known to be different.  */
>> > +  if (!__builtin_constant_p (inline_fn0 != inline_fn1))
>> > +    abort ();
>> > +  if (inline_fn0 == inline_fn1)
>> > +    abort ();
>> > +  return 0;
>> > +}

> On December 4, 2014 6:34:18 PM CET, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> 
> >> I think you want get_addr_base_and_unit_offset here.  But I really
> >wonder
> >
> >I copied what I found in tree-ssa-alias. The differenc eis that
> >get_addr_base_and_unit_offset won't give a range for variable sized
> >accesses right?
> 
> Yes. But OTOH you could also use non_aliasing_component_refs?

Hmm, nonoverlaping_component_refs tests strict_aliasing at beggining. Does it
depend on alias sets? In that case I think it may not be safe.  One can use 
component ref of one type to check if memory is unused and use it in different type
I think.
> 
> >> what cases this code catches that the code in fold_comparison you
> >touched
> >> above does not?  (apart from previously being bogus in different kind
> >of
> >> ways)
> >> 
> >> So I'd rather remove the code in fold_binary.
> >
> >Well, the code in fold_binary only handles comparsions where base
> >addresses are known
> >to be different. I.e. &a==&b returning false.
> >All the other cases are handled here.  I.. &a==&a or &a[5]==&b[7] etc.
> >Perhaps the code should be in fold_comparison?
> 
> Yes, I think it should be in one place.

I think too.  I actually found the fold_comparison code only while constructing a testcase.
I think it belongs there, so I will move all the logic there.

Honza

> 
> Richard.
> 
> >Honza
> >> 
> >> Thanks,
> >> Richard.
> >> 
> >> > +         offset_int offset0 = hwi_offset0;
> >> > +         offset_int offset1 = hwi_offset1;
> >> > +
> >> > +         /* Add constant offset of MEM_REF to OFFSET, if possible.
> > */
> >> > +         if (base0 && TREE_CODE (base0) == MEM_REF
> >> > +             && TREE_CODE (TREE_OPERAND (base0, 1)) ==
> >INTEGER_CST)
> >> > +           {
> >> > +             offset0 += wi::lshift (mem_ref_offset (base0),
> >LOG2_BITS_PER_UNIT);
> >> > +             base0 = TREE_OPERAND (base0, 0);
> >> > +           }
> >> > +         if (base1 && TREE_CODE (base1) == MEM_REF
> >> > +             && TREE_CODE (TREE_OPERAND (base1, 1)) ==
> >INTEGER_CST)
> >> > +           {
> >> > +             offset1 += wi::lshift (mem_ref_offset (base1),
> >LOG2_BITS_PER_UNIT);
> >> > +             base1 = TREE_OPERAND (base1, 0);
> >> > +           }
> >> > +         /* If both offsets of MEM_REF are the same, just ignore
> >them.  */
> >> > +         if (base0 && base1
> >> > +             && TREE_CODE (base0) == MEM_REF
> >> > +             && TREE_CODE (base1) == MEM_REF
> >> > +             && operand_equal_p (TREE_OPERAND (base0, 1),
> >TREE_OPERAND (base1, 1), 0))
> >> > +           {
> >> > +             base0 = TREE_OPERAND (base0, 0);
> >> > +             base1 = TREE_OPERAND (base1, 1);
> >> > +           }
> >> > +         /* If we see MEM_REF with variable offset, just modify
> >MAX_SIZE to declare that
> >> > +            sizes are unknown.  We can still prove that bases
> >points to different memory
> >> > +            locations.  */
> >> > +         if (base0 && TREE_CODE (base0) == MEM_REF)
> >> > +           {
> >> > +             base0 = TREE_OPERAND (base0, 0);
> >> > +             max_size0 = -1;
> >> > +           }
> >> > +         if (base1 && TREE_CODE (base1) == MEM_REF)
> >> > +           {
> >> > +             base1 = TREE_OPERAND (base1, 0);
> >> > +             max_size1 = -1;
> >> > +           }
> >> > +
> >> > +         /* If bases are equal or they are both declarations, we
> >can disprove equivalency by
> >> > +            proving that offsets are different.  */
> >> > +         if (base0 && base1 && (base0 == base1 || (DECL_P (base0)
> >&& DECL_P (base1))))
> >> > +           {
> >> > +             if ((wi::ltu_p (offset0 + max_size0 - size0, offset1)
> >> > +                  || (wi::ltu_p (offset1 + max_size1 - size1,
> >offset0)))
> >> > +                 && max_size0 != -1 && max_size1 != -1
> >> > +                 && size0 != -1 && size1 != -1)
> >> > +               return constant_boolean_node (code != EQ_EXPR,
> >type);
> >> > +           }
> >> > +         /* If bases are equal, then addresses are equal if
> >offsets are.
> >> > +            We can work hader here for non-constant offsets.  */
> >> > +         if (base0 && base0 == base1)
> >> > +           {
> >> > +             if (base0 == base1
> >> > +                 && size0 != -1 && size1 != -1
> >> > +                 && (max_size0 == size0) && (max_size1 == size1))
> >> > +               return constant_boolean_node (code == EQ_EXPR,
> >type);
> >> > +           }
> >> > +
> >> > +         if (base0 && base1 && DECL_P (base0) && DECL_P (base1))
> >> > +           {
> >> > +             bool in_symtab0 = decl_in_symtab_p (base0);
> >> > +             bool in_symtab1 = decl_in_symtab_p (base1);
> >> > +
> >> > +             /* Symtab and non-symtab declarations never overlap. 
> >*/
> >> > +             if (in_symtab0 != in_symtab1)
> >> > +               return constant_boolean_node (code != EQ_EXPR,
> >type);
> >> > +             /* Non-symtab nodes never have aliases: different
> >declaration means
> >> > +                different memory object.  */
> >> > +             if (!in_symtab0)
> >> > +               {
> >> > +                 if (base0 != base1 && TREE_CODE (base0) !=
> >TREE_CODE (base1))
> >> > +                   return constant_boolean_node (code != EQ_EXPR,
> >type);
> >> > +               }
> >> > +             else
> >> > +               {
> >> > +                 struct symtab_node *symbol0 =
> >symtab_node::get_create (base0);
> >> > +                 struct symtab_node *symbol1 =
> >symtab_node::get_create (base1);
> >> > +                 int cmp = symbol0->equal_address_to (symbol1);
> >> > +
> >> > +                 if (cmp == 0)
> >> > +                   return constant_boolean_node (code != EQ_EXPR,
> >type);
> >> > +                 if (cmp == 1
> >> > +                     && size0 != -1 && size1 != -1
> >> > +                     && (max_size0 == size0) && (max_size1 ==
> >size1))
> >> > +                   return constant_boolean_node (code == EQ_EXPR,
> >type);
> >> > +               }
> >> > +           }
> >> > +       }
> >> > +
> >> >        /* Transform comparisons of the form X +- Y CMP X to Y CMP
> >0.  */
> >> >        if ((TREE_CODE (arg0) == PLUS_EXPR
> >> >            || TREE_CODE (arg0) == POINTER_PLUS_EXPR
> >> > Index: symtab.c
> >> > ===================================================================
> >> > --- symtab.c    (revision 218286)
> >> > +++ symtab.c    (working copy)
> >> > @@ -1860,3 +1860,90 @@
> >> >      return true;
> >> >    return false;
> >> >  }
> >> > +
> >> > +/* Return 0 if symbol is known to have different address than S2,
> >> > +   Return 1 if symbol is known to have same address as S2,
> >> > +   return 2 otherwise.   */
> >> > +int
> >> > +symtab_node::equal_address_to (symtab_node *s2)
> >> > +{
> >> > +  enum availability avail1, avail2;
> >> > +
> >> > +  /* A Shortcut: equivalent symbols are always equivalent.  */
> >> > +  if (this == s2)
> >> > +    return 1;
> >> > +
> >> > +  /* For non-interposable aliases, lookup and compare their actual
> >definitions.
> >> > +     Also check if the symbol needs to bind to given definition. 
> >*/
> >> > +  symtab_node *rs1 = ultimate_alias_target (&avail1);
> >> > +  symtab_node *rs2 = s2->ultimate_alias_target (&avail2);
> >> > +  bool binds_local1 = rs1->analyzed && decl_binds_to_current_def_p
> >(this->decl);
> >> > +  bool binds_local2 = rs2->analyzed && decl_binds_to_current_def_p
> >(s2->decl);
> >> > +  bool really_binds_local1 = binds_local1;
> >> > +  bool really_binds_local2 = binds_local2;
> >> > +
> >> > +  /* Addresses of vtables and virtual functions can not be used by
> >user
> >> > +     code and are used only within speculation.  In this case we
> >may make
> >> > +     symbol equivalent to its alias even if interposition may
> >break this
> >> > +     rule.  Doing so will allow us to turn speculative inlining
> >into
> >> > +     non-speculative more agressively.  */
> >> > +  if (DECL_VIRTUAL_P (this->decl) && avail1 >= AVAIL_AVAILABLE)
> >> > +    binds_local1 = true;
> >> > +  if (DECL_VIRTUAL_P (s2->decl) && avail2 >= AVAIL_AVAILABLE)
> >> > +    binds_local2 = true;
> >> > +
> >> > +  /* If both definitions are available we know that even if they
> >are bound
> >> > +     to other unit they must be defined same way and therefore we
> >can use
> >> > +     equivalence test.  */
> >> > +  if (rs1 != rs2 && avail1 >= AVAIL_AVAILABLE && avail2 >=
> >AVAIL_AVAILABLE)
> >> > +    binds_local1 = binds_local2 = true;
> >> > +
> >> > +  if ((binds_local1 ? rs1 : this)
> >> > +       == (binds_local2 ? rs2 : s2))
> >> > +    {
> >> > +      /* We made use of the fact that alias is not weak.  */
> >> > +      if (binds_local1 && rs1 != this)
> >> > +        refuse_visibility_changes = true;
> >> > +      if (binds_local2 && rs2 != s2)
> >> > +        s2->refuse_visibility_changes = true;
> >> > +      return 1;
> >> > +    }
> >> > +
> >> > +  /* If both symbols may resolve to NULL, we can not really prove
> >them different.  */
> >> > +  if (!nonzero_address () && !s2->nonzero_address ())
> >> > +    return 2;
> >> > +
> >> > +  /* Except for NULL, functions and variables never overlap.  */
> >> > +  if (TREE_CODE (decl) != TREE_CODE (s2->decl))
> >> > +    return 0;
> >> > +
> >> > +  /* If one of the symbols is unresolved alias, punt.  */
> >> > +  if (rs1->alias || rs2->alias)
> >> > +    return 2;
> >> > +
> >> > +  /* If we have a non-interposale definition of at least one of
> >the symbols
> >> > +     and the other symbol is different, we know other unit can not
> >interpose
> >> > +     it to the first symbol; all aliases of the definition needs
> >to be
> >> > +     present in the current unit.  */
> >> > +  if (((really_binds_local1 || really_binds_local2)
> >> > +      /* If we have both definitions and they are different, we
> >know they
> >> > +        will be different even in units they binds to.  */
> >> > +       || (binds_local1 && binds_local2))
> >> > +      && rs1 != rs2)
> >> > +    {
> >> > +      /* We make use of the fact that one symbol is not alias of
> >the other
> >> > +        and that the definition is non-interposable.  */
> >> > +      refuse_visibility_changes = true;
> >> > +      s2->refuse_visibility_changes = true;
> >> > +      rs1->refuse_visibility_changes = true;
> >> > +      rs2->refuse_visibility_changes = true;
> >> > +      return 0;
> >> > +    }
> >> > +
> >> > +  /* TODO: Alias oracle basically assume that addresses of global
> >variables
> >> > +     are different unless they are declared as alias of one to
> >another.
> >> > +     We probably should be consistent and use this fact here, too,
> >and update
> >> > +     alias oracle to use this predicate.  */
> >> > +
> >> > +  return 2;
> >> > +}
> >> > Index: testsuite/gcc.dg/addr_equal-1.c
> >> > ===================================================================
> >> > --- testsuite/gcc.dg/addr_equal-1.c     (revision 0)
> >> > +++ testsuite/gcc.dg/addr_equal-1.c     (working copy)
> >> > @@ -0,0 +1,107 @@
> >> > +/* { dg-do run } */
> >> > +/* { dg-require-weak "" } */
> >> > +/* { dg-require-alias "" } */
> >> > +/* { dg-options "-O2" } */
> >> > +void abort (void);
> >> > +extern int undef_var0, undef_var1;
> >> > +extern __attribute__ ((weak)) int weak_undef_var0;
> >> > +extern __attribute__ ((weak)) int weak_undef_var1;
> >> > +__attribute__ ((weak)) int weak_def_var0;
> >> > +int def_var0=0, def_var1=0;
> >> > +static int alias_var0 __attribute__ ((alias("def_var0")));
> >> > +extern int weak_alias_var0 __attribute__ ((alias("def_var0")))
> >__attribute__ ((weak));
> >> > +void undef_fn0(void);
> >> > +void undef_fn1(void);
> >> > +void def_fn0(void)
> >> > +{
> >> > +}
> >> > +void def_fn1(void)
> >> > +{
> >> > +}
> >> > +__attribute__ ((weak))
> >> > +void weak_def_fn0(void)
> >> > +{
> >> > +}
> >> > +__attribute__ ((weak))
> >> > +void weak_def_fn1(void)
> >> > +{
> >> > +}
> >> > +__attribute__ ((weak)) void weak_undef_fn0(void);
> >> > +
> >> > +inline
> >> > +void inline_fn0(void)
> >> > +{
> >> > +}
> >> > +inline
> >> > +void inline_fn1(void)
> >> > +{
> >> > +}
> >> > +
> >> > +int
> >> > +main(int argc, char **argv)
> >> > +{
> >> > +  /* Two definitions are always different unless they can be
> >interposed.  */
> >> > +  if (!__builtin_constant_p (def_fn0 == def_fn1))
> >> > +    abort();
> >> > +  if (def_fn0 == def_fn1)
> >> > +    abort();
> >> > +
> >> > +  if (!__builtin_constant_p (&def_var0 == &def_var1))
> >> > +    abort();
> >> > +  if (&def_var0 == &def_var1)
> >> > +    abort();
> >> > +
> >> > +  /* Same symbol is the same no matter on interposition.  */
> >> > +  if (!__builtin_constant_p (undef_fn0 != undef_fn0))
> >> > +    abort ();
> >> > +  if (undef_fn0 != undef_fn0)
> >> > +    abort ();
> >> > +
> >> > +  /* Do not get confused by same offset.  */
> >> > +  if (!__builtin_constant_p (&undef_var0 + argc != &undef_var0 +
> >argc))
> >> > +    abort ();
> >> > +  if (&undef_var0 + argc != &undef_var0 + argc)
> >> > +    abort ();
> >> > +
> >> > +  /* Alias and its target is equivalent unless one of them can be
> >interposed.  */
> >> > +  if (!__builtin_constant_p (&def_var0 != &alias_var0))
> >> > +    abort ();
> >> > +  if (&def_var0 != &alias_var0 )
> >> > +    abort ();
> >> > +
> >> > +  if (__builtin_constant_p (&def_var0 != &weak_alias_var0))
> >> > +    abort ();
> >> > +  if (&def_var0 != &weak_alias_var0)
> >> > +    abort ();
> >> > +
> >> > +  /* Weak definitions may be both NULL.  */
> >> > +  if (__builtin_constant_p ((void *)weak_undef_fn0 == (void
> >*)&weak_undef_var0))
> >> > +    abort ();
> >> > +  if ((void *)weak_undef_fn0 != (void *)&weak_undef_var0)
> >> > +    abort ();
> >> > +
> >> > +  /* Different offsets makes it safe to assume addresses are
> >different.  */
> >> > +  if (!__builtin_constant_p ((char *)weak_undef_fn0 + 4 != (char
> >*)&weak_undef_var1 + 8))
> >> > +    abort ();
> >> > +  if ((char *)weak_undef_fn0 + 4 == (char *)&weak_undef_var1 + 8)
> >> > +    abort ();
> >> > +
> >> > +  /* Variables and functions do not share same memory locations
> >otherwise.  */
> >> > +  if (!__builtin_constant_p ((void *)undef_fn0 == (void
> >*)&undef_var0))
> >> > +    abort ();
> >> > +  if ((void *)undef_fn0 == (void *)&undef_var0)
> >> > +    abort ();
> >> > +
> >> > +  /* This works for cases where one object is just weakly defined,
> >too.  */
> >> > +  if (!__builtin_constant_p ((void *)weak_undef_fn0 == (void
> >*)&weak_def_var0))
> >> > +    abort ();
> >> > +  if ((void *)weak_undef_fn0 == (void *)&weak_def_var0)
> >> > +    abort ();
> >> > +
> >> > +  /* Inline functions are known to be different.  */
> >> > +  if (!__builtin_constant_p (inline_fn0 != inline_fn1))
> >> > +    abort ();
> >> > +  if (inline_fn0 == inline_fn1)
> >> > +    abort ();
> >> > +  return 0;
> >> > +}
>