| Message ID | 1333594591.19102.40.camel@gnopaine |
|---|---|
| State | New |
| Headers | show |
On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: > There seems to be tacit agreement that the vector tests should use > -fno-common on all targets to avoid the recent spate of failures (see > discussion in 52571 and 52603). > OK for trunk? Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working.
On Thu, Apr 5, 2012 at 6:22 AM, Mike Stump <mikestump@comcast.net> wrote: > On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: >> There seems to be tacit agreement that the vector tests should use >> -fno-common on all targets to avoid the recent spate of failures (see >> discussion in 52571 and 52603). > >> OK for trunk? > > Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working. Ian, you are the "source" of all of these problems. While I did not notice any degradations in SPEC (on x86) with handling commons "correctly" now, the fact that our testsuite needs -fno-common to make things vectorizable shows that users might be impacted negatively by this, which is only a real problem in corner cases. Why can the link editor not promote the definitions alignment when merging with a common with bigger alignment? Richard.
On Thu, 2012-04-05 at 11:30 +0200, Richard Guenther wrote: > On Thu, Apr 5, 2012 at 6:22 AM, Mike Stump <mikestump@comcast.net> wrote: > > On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: > >> There seems to be tacit agreement that the vector tests should use > >> -fno-common on all targets to avoid the recent spate of failures (see > >> discussion in 52571 and 52603). > > > >> OK for trunk? > > > > Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working. > > Ian, you are the "source" of all of these problems. While I did not notice > any degradations in SPEC (on x86) with handling commons "correctly" > now, the fact > that our testsuite needs -fno-common to make things vectorizable shows > that users might be impacted negatively by this, which is only a real problem > in corner cases. Why can the link editor not promote the definitions alignment > when merging with a common with bigger alignment? > > Richard. > Follow-up question: Should -ftree-vectorize imply -fno-common in the short term? Thanks, Bill
On Thu, Apr 5, 2012 at 1:59 PM, William J. Schmidt <wschmidt@linux.vnet.ibm.com> wrote: > On Thu, 2012-04-05 at 11:30 +0200, Richard Guenther wrote: >> On Thu, Apr 5, 2012 at 6:22 AM, Mike Stump <mikestump@comcast.net> wrote: >> > On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: >> >> There seems to be tacit agreement that the vector tests should use >> >> -fno-common on all targets to avoid the recent spate of failures (see >> >> discussion in 52571 and 52603). >> > >> >> OK for trunk? >> > >> > Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working. >> >> Ian, you are the "source" of all of these problems. While I did not notice >> any degradations in SPEC (on x86) with handling commons "correctly" >> now, the fact >> that our testsuite needs -fno-common to make things vectorizable shows >> that users might be impacted negatively by this, which is only a real problem >> in corner cases. Why can the link editor not promote the definitions alignment >> when merging with a common with bigger alignment? >> >> Richard. >> > > Follow-up question: Should -ftree-vectorize imply -fno-common in the > short term? That's probably more a C language question - you would get valid C rejected with -fno-common. But maybe -ftree-vectorize should suggest -fno-common when it encounters a case it would like to promote alignment for. Not sure if for example Fortran would ever work with -fno-common though. Richard. > Thanks, > Bill >
On Apr 5, 2012, at 2:30 AM, Richard Guenther wrote: > Why can the link editor not promote the definitions alignment > when merging with a common with bigger alignment? The problem is that when a common symbol is upped in alignment, but then not chosen by ld (or worse, by the dynamic linker), but the codegen that assumes a larger alignment is used, then you realize the alignment of the data actually selected must be retroactively upped as well. This means, the alignment in existing .a files, .o files, and .so files. The .a and .o files can be easily solved by simply requiring the compile of the world after upgrading all linkers (static and dynamic) to only have -fdata-sections. For .so files, well, that's above my pay grade. Darwin, I'll note, can manage the upping on the size and alignment, but only between all the commons, not a hard definition behind it.
Richard Guenther <richard.guenther@gmail.com> writes: > On Thu, Apr 5, 2012 at 6:22 AM, Mike Stump <mikestump@comcast.net> wrote: >> On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: >>> There seems to be tacit agreement that the vector tests should use >>> -fno-common on all targets to avoid the recent spate of failures (see >>> discussion in 52571 and 52603). >> >>> OK for trunk? >> >> Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working. > > Ian, you are the "source" of all of these problems. While I did not notice > any degradations in SPEC (on x86) with handling commons "correctly" > now, the fact > that our testsuite needs -fno-common to make things vectorizable shows > that users might be impacted negatively by this, which is only a real problem > in corner cases. Why can the link editor not promote the definitions alignment > when merging with a common with bigger alignment? The defined symbol will be embedded in a data section along with other defined data symbols, at some offset O from the start of the section. The data section will have a required alignment A. It is entirely possible that there is no way to honor A and also ensure that A+O is aligned as requested by the common symbol. This whole issue only applies to vectorization involving global symbols defined with no initializer in C, when vectorization requires an alignment greater than that implied by the type of the symbol. It does not affect function arguments or local variables. It does not affect C++. Is vectorization of uninitialized global symbols really a common case? Another approach we could use is to say that when the vectorizer wants to increase the alignment of a common symbol, it would force the symbol to be defined rather than common. That would be safe, as it would lead to a multiple-definition error at link time in cases where it would be unsafe. I would be fine with making the compiler work that way for C, although of course there would have to be an option to disable it. I don't know enough about Fortran to know whether the same issues arise there. Perhaps in Fortran a common symbol is always a common symbol and can never be a defined symbol. If that is the case then for Fortran I think it would be safe to change the alignment of the common symbol. Of course we would need to have some way to indicate that in the middle-end--DECL_ALWAYS_COMMON or something. Ian
On Fri, Apr 6, 2012 at 12:15 AM, Ian Lance Taylor <iant@google.com> wrote: > Richard Guenther <richard.guenther@gmail.com> writes: > >> On Thu, Apr 5, 2012 at 6:22 AM, Mike Stump <mikestump@comcast.net> wrote: >>> On Apr 4, 2012, at 7:56 PM, William J. Schmidt wrote: >>>> There seems to be tacit agreement that the vector tests should use >>>> -fno-common on all targets to avoid the recent spate of failures (see >>>> discussion in 52571 and 52603). >>> >>>> OK for trunk? >>> >>> Ok. Any other solution I think will be real work and we shouldn't loose the testing between now and then by not having the test cases working. >> >> Ian, you are the "source" of all of these problems. While I did not notice >> any degradations in SPEC (on x86) with handling commons "correctly" >> now, the fact >> that our testsuite needs -fno-common to make things vectorizable shows >> that users might be impacted negatively by this, which is only a real problem >> in corner cases. Why can the link editor not promote the definitions alignment >> when merging with a common with bigger alignment? > > The defined symbol will be embedded in a data section along with other > defined data symbols, at some offset O from the start of the section. > The data section will have a required alignment A. It is entirely > possible that there is no way to honor A and also ensure that A+O is > aligned as requested by the common symbol. > > This whole issue only applies to vectorization involving global symbols > defined with no initializer in C, when vectorization requires an > alignment greater than that implied by the type of the symbol. It does > not affect function arguments or local variables. It does not affect > C++. Is vectorization of uninitialized global symbols really a common > case? > > Another approach we could use is to say that when the vectorizer wants > to increase the alignment of a common symbol, it would force the symbol > to be defined rather than common. That would be safe, as it would lead > to a multiple-definition error at link time in cases where it would be > unsafe. I would be fine with making the compiler work that way for C, > although of course there would have to be an option to disable it. That would work fine at least with LTO - though I'm not sure the linker-plugin gives enough feedback that a common is not overridden by an external definition. Maybe it would be a good idea to promote all commons to definitions with LTO (where possible, of course). > I don't know enough about Fortran to know whether the same issues arise > there. Perhaps in Fortran a common symbol is always a common symbol and > can never be a defined symbol. If that is the case then for Fortran I > think it would be safe to change the alignment of the common symbol. Of > course we would need to have some way to indicate that in the > middle-end--DECL_ALWAYS_COMMON or something. I don't know either. OTOH commons are records, so the vectorizer cannot change alignment of a common sub-variable - so in this case it would be better if the frontend would align the common to BIGGEST_ALIGNMENT or so. Richard. > Ian
Index: gcc/testsuite/gcc.dg/vect/costmodel/ppc/ppc-costmodel-vect.exp =================================================================== --- gcc/testsuite/gcc.dg/vect/costmodel/ppc/ppc-costmodel-vect.exp (revision 186108) +++ gcc/testsuite/gcc.dg/vect/costmodel/ppc/ppc-costmodel-vect.exp (working copy) @@ -34,7 +34,7 @@ if ![is-effective-target powerpc_altivec_ok] { set DEFAULT_VECTCFLAGS "" # These flags are used for all targets. -lappend DEFAULT_VECTCFLAGS "-O2" "-ftree-vectorize" "-fvect-cost-model" +lappend DEFAULT_VECTCFLAGS "-O2" "-ftree-vectorize" "-fvect-cost-model" "-fno-common" # If the target system supports vector instructions, the default action # for a test is 'run', otherwise it's 'compile'. Save current default. Index: gcc/testsuite/gcc.dg/vect/vect.exp =================================================================== --- gcc/testsuite/gcc.dg/vect/vect.exp (revision 186108) +++ gcc/testsuite/gcc.dg/vect/vect.exp (working copy) @@ -40,7 +40,7 @@ if ![check_vect_support_and_set_flags] { } # These flags are used for all targets. -lappend DEFAULT_VECTCFLAGS "-ftree-vectorize" "-fno-vect-cost-model" +lappend DEFAULT_VECTCFLAGS "-ftree-vectorize" "-fno-vect-cost-model" "-fno-common" # Initialize `dg'. dg-init
There seems to be tacit agreement that the vector tests should use -fno-common on all targets to avoid the recent spate of failures (see discussion in 52571 and 52603). This patch (proposed by Dominique D'Humieures) does just that. I agreed to shepherd the patch through. I've verified that it removes the failures for powerpc64-linux. Various others have verified for arm, sparc, and darwin. OK for trunk? Thanks, Bill gcc/testsuite: 2012-04-04 Bill Schmidt <wschmidt@linux.vnet.ibm.com> Dominique D'Humieures <dominiq@lps.ens.fr> PR testsuite/52614 * gcc.dg/vect/vect.exp: Use -fno-common on all targets. * gcc.dg/vect/costmodel/ppc/ppc-costmodel-vect.exp: Likewise.