On December 4, 2014 6:34:18 PM CET, Jan Hubicka <hubi...@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; >> > +}