| Message ID | CAFULd4bkJNSaaqoxxC5ttCf8pZrvkts6jYKNPXiPAqqR2mHM0w@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: >Hello! > >There is a logic error in Honza's patch "Transparent alias suport part >10" [1]. The part in memrefs_conflict_p should be changed to: > >- /* If decls are different or we know by offsets that there is no >overlap, >- we win. */ >- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >+ /* If decls are different and we know by offsets that >+ there is no overlap, we win. */ >+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) > return 0; >- /* Decls may or may not be different and offsets overlap....*/ >+ /* Decls are different and offsets overlap....*/ > >Even if decls are different, their offsets shouldn't overlap! Comparing offsets of different decls does not make sense. >Addresses with alignment ANDs depend on increased xsize and ysize, so >no wonder gcc fails to bootstrap on alpha. Alpha also fails to honor the C++ memory model and introduces store data races. I wonder if alpha needs to simply pad all decls appropriately. Maybe also finally time to ditch this arch (Or affected sub-arch)? Richard. > >In addition to this, the check for SYMBOL_REFs in base_alias_check >should be moved after checks for addresses. > >The patch also adds some simplification. If >symtab_address::equal_address_to returns -1 for "unknown" (as is >otherwise customary throughout the sources), we can simplify >compare_base_decls a bit. > >2015-12-23 Uros Bizjak <ubizjak@gmail.com> > > PR middle-end/68999 > * symtab.c (symtab_node::equal_address_to): Return -1 instead of 2 > if we can't determine address equivalence. > * alias.c (compare_base_decl): Update for changed return value of > symtab_node::equal_address_to. > (memrefs_conflict_p): Return 0 when decls are different > and offsets don't overlap. > (base_alias_check): Move check for addresses with alignment ANDs > before the call for compare_base_decls. > >The patch was bootstrapped and regression tested on x86_64-linux-gnu >{,-m32} and alpha-linux-gnu [2], which is a massive user of addresses >with alignment ANDs > >OK for mainline? > >[1] https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01076.html >[2] https://gcc.gnu.org/ml/gcc-testresults/2015-12/msg02372.html > >Uros.
On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: >>Hello! >> >>There is a logic error in Honza's patch "Transparent alias suport part >>10" [1]. The part in memrefs_conflict_p should be changed to: >> >>- /* If decls are different or we know by offsets that there is no >>overlap, >>- we win. */ >>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >>+ /* If decls are different and we know by offsets that >>+ there is no overlap, we win. */ >>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >> return 0; >>- /* Decls may or may not be different and offsets overlap....*/ >>+ /* Decls are different and offsets overlap....*/ >> >>Even if decls are different, their offsets shouldn't overlap! > > Comparing offsets of different decls does not make sense. Please consider two variables, say bytes, located at the neighbouring locations, not aligned to 8 bytes. Now, first byte is to be written by first using 8-byte aligned DImode load, change the byte and write full DImode value back. When second byte is to be written, the same procedure is used, but its AND aligned address aliases the location of the first byte. So, we have: - load the first 8-byte word from 8-byte aligned address, enclosing the first variable - change the byte - store the first 8-byte word to 8-byte aligned address - load the second 8-byte word from 8-byte aligned address (that aliases with the address above), enclosing the second variable - change the byte - store the second 8-byte word to 8-byte aligned address Now scheduler comes into play and swaps line 3 and 4: - load the first 8-byte word from 8-byte aligned address, enclosing the first variable - change the byte - load the second 8-byte word from 8-byte aligned address (that aliases with the address above), enclosing the second variable - store the first 8-byte word to 8-byte aligned address - change the byte - store the second 8-byte word to 8-byte aligned address So, the second store killed the value of the first store. Using the proposed logic, two variables aliases each other by xsize that is increased according to AND operand and ysize, increased in the same way. >>Addresses with alignment ANDs depend on increased xsize and ysize, so >>no wonder gcc fails to bootstrap on alpha. > > Alpha also fails to honor the C++ memory model and introduces store data races. > I wonder if alpha needs to simply pad all decls appropriately. alphaev68 works in the correct way, but it still uses AND addresses. Uros. > Maybe also finally time to ditch this arch > (Or affected sub-arch)? > > Richard. > >> >>In addition to this, the check for SYMBOL_REFs in base_alias_check >>should be moved after checks for addresses. >> >>The patch also adds some simplification. If >>symtab_address::equal_address_to returns -1 for "unknown" (as is >>otherwise customary throughout the sources), we can simplify >>compare_base_decls a bit. >> >>2015-12-23 Uros Bizjak <ubizjak@gmail.com> >> >> PR middle-end/68999 >> * symtab.c (symtab_node::equal_address_to): Return -1 instead of 2 >> if we can't determine address equivalence. >> * alias.c (compare_base_decl): Update for changed return value of >> symtab_node::equal_address_to. >> (memrefs_conflict_p): Return 0 when decls are different >> and offsets don't overlap. >> (base_alias_check): Move check for addresses with alignment ANDs >> before the call for compare_base_decls. >> >>The patch was bootstrapped and regression tested on x86_64-linux-gnu >>{,-m32} and alpha-linux-gnu [2], which is a massive user of addresses >>with alignment ANDs >> >>OK for mainline? >> >>[1] https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01076.html >>[2] https://gcc.gnu.org/ml/gcc-testresults/2015-12/msg02372.html >> >>Uros. > >
On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: >>Hello! >> >>There is a logic error in Honza's patch "Transparent alias suport part >>10" [1]. The part in memrefs_conflict_p should be changed to: >> >>- /* If decls are different or we know by offsets that there is no >>overlap, >>- we win. */ >>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >>+ /* If decls are different and we know by offsets that >>+ there is no overlap, we win. */ >>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >> return 0; >>- /* Decls may or may not be different and offsets overlap....*/ >>+ /* Decls are different and offsets overlap....*/ >> >>Even if decls are different, their offsets shouldn't overlap! > > Comparing offsets of different decls does not make sense. Uh, yes, some more eyeballing was needed, but you are right. Is there a way to detect aliasing in case AND addresses are involved? Probably we need something like in base_alias_check, where: --cut here-- /* The base addresses are different expressions. If they are not accessed via AND, there is no conflict. We can bring knowledge of object alignment into play here. For example, on alpha, "char a, b;" can alias one another, though "char a; long b;" cannot. AND addesses may implicitly alias surrounding objects; i.e. unaligned access in DImode via AND address can alias all surrounding object types except those with aligment 8 or higher. */ if (GET_CODE (x) == AND && GET_CODE (y) == AND) return 1; if (GET_CODE (x) == AND && (!CONST_INT_P (XEXP (x, 1)) || (int) GET_MODE_UNIT_SIZE (y_mode) < -INTVAL (XEXP (x, 1)))) return 1; if (GET_CODE (y) == AND && (!CONST_INT_P (XEXP (y, 1)) || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1)))) return 1; --cut here- Uros.
On December 23, 2015 5:58:07 PM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: >On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener ><richard.guenther@gmail.com> wrote: >> On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak ><ubizjak@gmail.com> wrote: >>>Hello! >>> >>>There is a logic error in Honza's patch "Transparent alias suport >part >>>10" [1]. The part in memrefs_conflict_p should be changed to: >>> >>>- /* If decls are different or we know by offsets that there is >no >>>overlap, >>>- we win. */ >>>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >>>+ /* If decls are different and we know by offsets that >>>+ there is no overlap, we win. */ >>>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >>> return 0; >>>- /* Decls may or may not be different and offsets overlap....*/ >>>+ /* Decls are different and offsets overlap....*/ >>> >>>Even if decls are different, their offsets shouldn't overlap! >> >> Comparing offsets of different decls does not make sense. > >Uh, yes, some more eyeballing was needed, but you are right. > >Is there a way to detect aliasing in case AND addresses are involved? > >Probably we need something like in base_alias_check, where: Yeah, and in that case just give up. Richard. >--cut here-- >/* The base addresses are different expressions. If they are not >accessed > via AND, there is no conflict. We can bring knowledge of object > alignment into play here. For example, on alpha, "char a, b;" can > alias one another, though "char a; long b;" cannot. AND addesses may > implicitly alias surrounding objects; i.e. unaligned access in DImode > via AND address can alias all surrounding object types except those > with aligment 8 or higher. */ > if (GET_CODE (x) == AND && GET_CODE (y) == AND) > return 1; > if (GET_CODE (x) == AND > && (!CONST_INT_P (XEXP (x, 1)) > || (int) GET_MODE_UNIT_SIZE (y_mode) < -INTVAL (XEXP (x, 1)))) > return 1; > if (GET_CODE (y) == AND > && (!CONST_INT_P (XEXP (y, 1)) > || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1)))) > return 1; >--cut here- > >Uros.
> On December 23, 2015 5:58:07 PM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: > >On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener > ><richard.guenther@gmail.com> wrote: > >> On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak > ><ubizjak@gmail.com> wrote: > >>>Hello! > >>> > >>>There is a logic error in Honza's patch "Transparent alias suport > >part > >>>10" [1]. The part in memrefs_conflict_p should be changed to: > >>> > >>>- /* If decls are different or we know by offsets that there is > >no > >>>overlap, > >>>- we win. */ > >>>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) > >>>+ /* If decls are different and we know by offsets that > >>>+ there is no overlap, we win. */ > >>>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) > >>> return 0; > >>>- /* Decls may or may not be different and offsets overlap....*/ > >>>+ /* Decls are different and offsets overlap....*/ > >>> > >>>Even if decls are different, their offsets shouldn't overlap! > >> > >> Comparing offsets of different decls does not make sense. > > > >Uh, yes, some more eyeballing was needed, but you are right. > > > >Is there a way to detect aliasing in case AND addresses are involved? > > > >Probably we need something like in base_alias_check, where: > > Yeah, and in that case just give up. Yep, I will look into it ASAP. comparing offsets of different decls is intended tohandle the case where one decl is foo and other bar. We do not know of foo is not alias of bar, but if offsets are different, we can still disambiguate This needs to be tought over WRT anchors anyway. Honza > > Richard. > > >--cut here-- > >/* The base addresses are different expressions. If they are not > >accessed > > via AND, there is no conflict. We can bring knowledge of object > > alignment into play here. For example, on alpha, "char a, b;" can > > alias one another, though "char a; long b;" cannot. AND addesses may > > implicitly alias surrounding objects; i.e. unaligned access in DImode > > via AND address can alias all surrounding object types except those > > with aligment 8 or higher. */ > > if (GET_CODE (x) == AND && GET_CODE (y) == AND) > > return 1; > > if (GET_CODE (x) == AND > > && (!CONST_INT_P (XEXP (x, 1)) > > || (int) GET_MODE_UNIT_SIZE (y_mode) < -INTVAL (XEXP (x, 1)))) > > return 1; > > if (GET_CODE (y) == AND > > && (!CONST_INT_P (XEXP (y, 1)) > > || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1)))) > > return 1; > >--cut here- > > > >Uros. >
On December 24, 2015 11:15:52 AM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote: >> On December 23, 2015 5:58:07 PM GMT+01:00, Uros Bizjak ><ubizjak@gmail.com> wrote: >> >On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener >> ><richard.guenther@gmail.com> wrote: >> >> On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak >> ><ubizjak@gmail.com> wrote: >> >>>Hello! >> >>> >> >>>There is a logic error in Honza's patch "Transparent alias suport >> >part >> >>>10" [1]. The part in memrefs_conflict_p should be changed to: >> >>> >> >>>- /* If decls are different or we know by offsets that there >is >> >no >> >>>overlap, >> >>>- we win. */ >> >>>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >> >>>+ /* If decls are different and we know by offsets that >> >>>+ there is no overlap, we win. */ >> >>>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >> >>> return 0; >> >>>- /* Decls may or may not be different and offsets >overlap....*/ >> >>>+ /* Decls are different and offsets overlap....*/ >> >>> >> >>>Even if decls are different, their offsets shouldn't overlap! >> >> >> >> Comparing offsets of different decls does not make sense. >> > >> >Uh, yes, some more eyeballing was needed, but you are right. >> > >> >Is there a way to detect aliasing in case AND addresses are >involved? >> > >> >Probably we need something like in base_alias_check, where: >> >> Yeah, and in that case just give up. > >Yep, I will look into it ASAP. >comparing offsets of different decls is intended tohandle the case >where one decl is foo and other bar. ... And they resolve to the same address. For alpha we want to handle the case where an address based on decl A aliases decl B which do not resolve to the same address. That's quite different and is generally _not_ allowed in the middle-end (apart from that pesky and-addresses). We do not know of foo is not alias >of bar, but if offsets are different, we can still disambiguate >This needs to be tought over WRT anchors anyway. > >Honza >> >> Richard. >> >> >--cut here-- >> >/* The base addresses are different expressions. If they are not >> >accessed >> > via AND, there is no conflict. We can bring knowledge of >object >> > alignment into play here. For example, on alpha, "char a, b;" >can >> > alias one another, though "char a; long b;" cannot. AND addesses >may >> > implicitly alias surrounding objects; i.e. unaligned access in >DImode >> > via AND address can alias all surrounding object types except >those >> > with aligment 8 or higher. */ >> > if (GET_CODE (x) == AND && GET_CODE (y) == AND) >> > return 1; >> > if (GET_CODE (x) == AND >> > && (!CONST_INT_P (XEXP (x, 1)) >> > || (int) GET_MODE_UNIT_SIZE (y_mode) < -INTVAL (XEXP (x, 1)))) >> > return 1; >> > if (GET_CODE (y) == AND >> > && (!CONST_INT_P (XEXP (y, 1)) >> > || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1)))) >> > return 1; >> >--cut here- >> > >> >Uros. >>
On Thu, Dec 24, 2015 at 12:57 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On December 24, 2015 11:15:52 AM GMT+01:00, Jan Hubicka <hubicka@ucw.cz> wrote: >>> On December 23, 2015 5:58:07 PM GMT+01:00, Uros Bizjak >><ubizjak@gmail.com> wrote: >>> >On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener >>> ><richard.guenther@gmail.com> wrote: >>> >> On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak >>> ><ubizjak@gmail.com> wrote: >>> >>>Hello! >>> >>> >>> >>>There is a logic error in Honza's patch "Transparent alias suport >>> >part >>> >>>10" [1]. The part in memrefs_conflict_p should be changed to: >>> >>> >>> >>>- /* If decls are different or we know by offsets that there >>is >>> >no >>> >>>overlap, >>> >>>- we win. */ >>> >>>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >>> >>>+ /* If decls are different and we know by offsets that >>> >>>+ there is no overlap, we win. */ >>> >>>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >>> >>> return 0; >>> >>>- /* Decls may or may not be different and offsets >>overlap....*/ >>> >>>+ /* Decls are different and offsets overlap....*/ >>> >>> >>> >>>Even if decls are different, their offsets shouldn't overlap! >>> >> >>> >> Comparing offsets of different decls does not make sense. >>> > >>> >Uh, yes, some more eyeballing was needed, but you are right. >>> > >>> >Is there a way to detect aliasing in case AND addresses are >>involved? >>> > >>> >Probably we need something like in base_alias_check, where: >>> >>> Yeah, and in that case just give up. >> >>Yep, I will look into it ASAP. >>comparing offsets of different decls is intended tohandle the case >>where one decl is foo and other bar. > > ... And they resolve to the same address. > > For alpha we want to handle the case where an address based on decl A aliases decl B which do not resolve to the same address. That's quite different and is generally _not_ allowed in the middle-end (apart from that pesky and-addresses). Please note that rs6000 also needs AND-addresses, at least following can be found in rs6000.c: /* Reload an offset address wrapped by an AND that represents the masking of the lower bits. Strip the outer AND and let reload convert the offset address into an indirect address. For VSX, force reload to create the address with an AND in a separate register, because we can't guarantee an altivec register will be used. */ if (VECTOR_MEM_ALTIVEC_P (mode) && GET_CODE (x) == AND && GET_CODE (XEXP (x, 0)) == PLUS && GET_CODE (XEXP (XEXP (x, 0), 0)) == REG && GET_CODE (XEXP (XEXP (x, 0), 1)) == CONST_INT && GET_CODE (XEXP (x, 1)) == CONST_INT && INTVAL (XEXP (x, 1)) == -16) and /* If this is an unaligned stvx/ldvx type address, discard the outer AND. */ if (VECTOR_MEM_ALTIVEC_P (mode) && GET_CODE (x) == AND && GET_CODE (XEXP (x, 1)) == CONST_INT && INTVAL (XEXP (x, 1)) == -16) so, I'm afraid that these annoyances will need to be handled until the end of time ... alpha just uses AND-addresses a lot. The code I have quoted a few messages ago is needed to say that aliasing can't be determined, and this is all we need for a reliable scheduling. Uros.
On Thu, Dec 24, 2015 at 11:15 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> On December 23, 2015 5:58:07 PM GMT+01:00, Uros Bizjak <ubizjak@gmail.com> wrote: >> >On Wed, Dec 23, 2015 at 2:39 PM, Richard Biener >> ><richard.guenther@gmail.com> wrote: >> >> On December 23, 2015 10:39:17 AM GMT+01:00, Uros Bizjak >> ><ubizjak@gmail.com> wrote: >> >>>Hello! >> >>> >> >>>There is a logic error in Honza's patch "Transparent alias suport >> >part >> >>>10" [1]. The part in memrefs_conflict_p should be changed to: >> >>> >> >>>- /* If decls are different or we know by offsets that there is >> >no >> >>>overlap, >> >>>- we win. */ >> >>>- if (!cmp || !offset_overlap_p (c, xsize, ysize)) >> >>>+ /* If decls are different and we know by offsets that >> >>>+ there is no overlap, we win. */ >> >>>+ if (!cmp && !offset_overlap_p (c, xsize, ysize)) >> >>> return 0; >> >>>- /* Decls may or may not be different and offsets overlap....*/ >> >>>+ /* Decls are different and offsets overlap....*/ >> >>> >> >>>Even if decls are different, their offsets shouldn't overlap! >> >> >> >> Comparing offsets of different decls does not make sense. >> > >> >Uh, yes, some more eyeballing was needed, but you are right. >> > >> >Is there a way to detect aliasing in case AND addresses are involved? >> > >> >Probably we need something like in base_alias_check, where: >> >> Yeah, and in that case just give up. > > Yep, I will look into it ASAP. > comparing offsets of different decls is intended tohandle the case > where one decl is foo and other bar. We do not know of foo is not alias > of bar, but if offsets are different, we can still disambiguate > This needs to be tought over WRT anchors anyway. Thanks, I will be glad to test the patch on alpha native bootstrap. Uros.
On 12/23/2015 02:39 AM, Uros Bizjak wrote: > Hello! > > There is a logic error in Honza's patch "Transparent alias suport part > 10" [1]. The part in memrefs_conflict_p should be changed to: > > - /* If decls are different or we know by offsets that there is no overlap, > - we win. */ > - if (!cmp || !offset_overlap_p (c, xsize, ysize)) > + /* If decls are different and we know by offsets that > + there is no overlap, we win. */ > + if (!cmp && !offset_overlap_p (c, xsize, ysize)) > return 0; > - /* Decls may or may not be different and offsets overlap....*/ > + /* Decls are different and offsets overlap....*/ > > Even if decls are different, their offsets shouldn't overlap! > Addresses with alignment ANDs depend on increased xsize and ysize, so > no wonder gcc fails to bootstrap on alpha. > > In addition to this, the check for SYMBOL_REFs in base_alias_check > should be moved after checks for addresses. > > The patch also adds some simplification. If > symtab_address::equal_address_to returns -1 for "unknown" (as is > otherwise customary throughout the sources), we can simplify > compare_base_decls a bit. > > 2015-12-23 Uros Bizjak <ubizjak@gmail.com> > > PR middle-end/68999 > * symtab.c (symtab_node::equal_address_to): Return -1 instead of 2 > if we can't determine address equivalence. > * alias.c (compare_base_decl): Update for changed return value of > symtab_node::equal_address_to. > (memrefs_conflict_p): Return 0 when decls are different > and offsets don't overlap. > (base_alias_check): Move check for addresses with alignment ANDs > before the call for compare_base_decls. So with the symtab and part of the alias bits in, I assume you're going to post the remainder of the fix separately, right? jeff
On Sat, Jan 9, 2016 at 12:37 AM, Jeff Law <law@redhat.com> wrote: > On 12/23/2015 02:39 AM, Uros Bizjak wrote: >> >> Hello! >> >> There is a logic error in Honza's patch "Transparent alias suport part >> 10" [1]. The part in memrefs_conflict_p should be changed to: >> >> - /* If decls are different or we know by offsets that there is no >> overlap, >> - we win. */ >> - if (!cmp || !offset_overlap_p (c, xsize, ysize)) >> + /* If decls are different and we know by offsets that >> + there is no overlap, we win. */ >> + if (!cmp && !offset_overlap_p (c, xsize, ysize)) >> return 0; >> - /* Decls may or may not be different and offsets overlap....*/ >> + /* Decls are different and offsets overlap....*/ >> >> Even if decls are different, their offsets shouldn't overlap! >> Addresses with alignment ANDs depend on increased xsize and ysize, so >> no wonder gcc fails to bootstrap on alpha. >> >> In addition to this, the check for SYMBOL_REFs in base_alias_check >> should be moved after checks for addresses. >> >> The patch also adds some simplification. If >> symtab_address::equal_address_to returns -1 for "unknown" (as is >> otherwise customary throughout the sources), we can simplify >> compare_base_decls a bit. >> >> 2015-12-23 Uros Bizjak <ubizjak@gmail.com> >> >> PR middle-end/68999 >> * symtab.c (symtab_node::equal_address_to): Return -1 instead of 2 >> if we can't determine address equivalence. >> * alias.c (compare_base_decl): Update for changed return value of >> symtab_node::equal_address_to. >> (memrefs_conflict_p): Return 0 when decls are different >> and offsets don't overlap. >> (base_alias_check): Move check for addresses with alignment ANDs >> before the call for compare_base_decls. > > So with the symtab and part of the alias bits in, I assume you're going to > post the remainder of the fix separately, right? Yes, the patch is already posted at [1]. [1] https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00436.html Uros.
Index: alias.c =================================================================== --- alias.c (revision 231896) +++ alias.c (working copy) @@ -2047,8 +2047,6 @@ compare_base_decls (tree base1, tree base2) return 0; ret = symtab_node::get_create (base1)->equal_address_to (symtab_node::get_create (base2), true); - if (ret == 2) - return -1; return ret; } @@ -2089,17 +2087,6 @@ base_alias_check (rtx x, rtx x_base, rtx y, rtx y_ if (rtx_equal_p (x_base, y_base)) return 1; - if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF) - { - tree x_decl = SYMBOL_REF_DECL (x_base); - tree y_decl = SYMBOL_REF_DECL (y_base); - - /* We can assume that no stores are made to labels. */ - if (!x_decl || !y_decl) - return 0; - return compare_base_decls (x_decl, y_decl) != 0; - } - /* The base addresses are different expressions. If they are not accessed via AND, there is no conflict. We can bring knowledge of object alignment into play here. For example, on alpha, "char a, b;" can @@ -2118,6 +2105,17 @@ base_alias_check (rtx x, rtx x_base, rtx y, rtx y_ || (int) GET_MODE_UNIT_SIZE (x_mode) < -INTVAL (XEXP (y, 1)))) return 1; + if (GET_CODE (x_base) == SYMBOL_REF && GET_CODE (y_base) == SYMBOL_REF) + { + tree x_decl = SYMBOL_REF_DECL (x_base); + tree y_decl = SYMBOL_REF_DECL (y_base); + + /* We can assume that no stores are made to labels. */ + if (!x_decl || !y_decl) + return 0; + return compare_base_decls (x_decl, y_decl) != 0; + } + /* Differing symbols not accessed via AND never alias. */ if (GET_CODE (x_base) != ADDRESS && GET_CODE (y_base) != ADDRESS) return 0; @@ -2340,11 +2338,11 @@ memrefs_conflict_p (int xsize, rtx x, int ysize, r /* If both decls are the same, decide by offsets. */ if (cmp == 1) return offset_overlap_p (c, xsize, ysize); - /* If decls are different or we know by offsets that there is no overlap, - we win. */ - if (!cmp || !offset_overlap_p (c, xsize, ysize)) + /* If decls are different and we know by offsets that + there is no overlap, we win. */ + if (!cmp && !offset_overlap_p (c, xsize, ysize)) return 0; - /* Decls may or may not be different and offsets overlap....*/ + /* Decls are different and offsets overlap....*/ return -1; } else if (rtx_equal_for_memref_p (x, y)) Index: symtab.c =================================================================== --- symtab.c (revision 231896) +++ symtab.c (working copy) @@ -1877,7 +1877,7 @@ symtab_node::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. + return -1 otherwise. If MEMORY_ACCESSED is true, assume that both memory pointer to THIS and S2 is going to be accessed. This eliminates the situations when @@ -1941,7 +1941,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo /* If both symbols may resolve to NULL, we can not really prove them different. */ if (!memory_accessed && !nonzero_address () && !s2->nonzero_address ()) - return 2; + return -1; /* Except for NULL, functions and variables never overlap. */ if (TREE_CODE (decl) != TREE_CODE (s2->decl)) @@ -1949,7 +1949,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo /* If one of the symbols is unresolved alias, punt. */ if (rs1->alias || rs2->alias) - return 2; + return -1; /* 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 @@ -1976,7 +1976,7 @@ symtab_node::equal_address_to (symtab_node *s2, bo We probably should be consistent and use this fact here, too, but for the moment return false only when we are called from the alias oracle. */ - return memory_accessed && rs1 != rs2 ? 0 : 2; + return memory_accessed && rs1 != rs2 ? 0 : -1; } /* Worker for call_for_symbol_and_aliases. */
Hello! There is a logic error in Honza's patch "Transparent alias suport part 10" [1]. The part in memrefs_conflict_p should be changed to: - /* If decls are different or we know by offsets that there is no overlap, - we win. */ - if (!cmp || !offset_overlap_p (c, xsize, ysize)) + /* If decls are different and we know by offsets that + there is no overlap, we win. */ + if (!cmp && !offset_overlap_p (c, xsize, ysize)) return 0; - /* Decls may or may not be different and offsets overlap....*/ + /* Decls are different and offsets overlap....*/ Even if decls are different, their offsets shouldn't overlap! Addresses with alignment ANDs depend on increased xsize and ysize, so no wonder gcc fails to bootstrap on alpha. In addition to this, the check for SYMBOL_REFs in base_alias_check should be moved after checks for addresses. The patch also adds some simplification. If symtab_address::equal_address_to returns -1 for "unknown" (as is otherwise customary throughout the sources), we can simplify compare_base_decls a bit. 2015-12-23 Uros Bizjak <ubizjak@gmail.com> PR middle-end/68999 * symtab.c (symtab_node::equal_address_to): Return -1 instead of 2 if we can't determine address equivalence. * alias.c (compare_base_decl): Update for changed return value of symtab_node::equal_address_to. (memrefs_conflict_p): Return 0 when decls are different and offsets don't overlap. (base_alias_check): Move check for addresses with alignment ANDs before the call for compare_base_decls. The patch was bootstrapped and regression tested on x86_64-linux-gnu {,-m32} and alpha-linux-gnu [2], which is a massive user of addresses with alignment ANDs OK for mainline? [1] https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01076.html [2] https://gcc.gnu.org/ml/gcc-testresults/2015-12/msg02372.html Uros.