Re: Fix folding of EQ/NE_EXPR WRT symtab aliases

[Richard Biener <richard dot guenther at gmail dot com>]

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;
>> > +}