| Message ID | yddwpd28fqd.fsf@CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
On Feb 7, 2017, at 2:20 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > No. In fact, I'd go for something like this: > > 2017-02-07 Dominik Vogt <vogt@linux.vnet.ibm.com> > Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > * g++.dg/tls/thread_local-order2.C: Only xfail execution on > *-*-solaris*. > > # HG changeset patch > # Parent 031bb7a327cc984d387a8ae64e7c65d4b8793731 > Only xfail g++.dg/tls/thread_local-order2.C on Solaris > > diff --git a/gcc/testsuite/g++.dg/tls/thread_local-order2.C b/gcc/testsuite/g++.dg/tls/thread_local-order2.C > --- a/gcc/testsuite/g++.dg/tls/thread_local-order2.C > +++ b/gcc/testsuite/g++.dg/tls/thread_local-order2.C > @@ -2,10 +2,11 @@ > // that isn't reverse order of construction. We need to move > // __cxa_thread_atexit into glibc to get this right. > > -// { dg-do run { xfail *-*-* } } > +// { dg-do run } > // { dg-require-effective-target c++11 } > // { dg-add-options tls } > // { dg-require-effective-target tls_runtime } > +// { dg-xfail-run-if "" { *-*-solaris* } } > > extern "C" void abort(); > extern "C" int printf (const char *, ...); > > This way one can easily add per-target PR references or explanations, > e.g. for darwin10 or others should they come up. > > Tested on i386-pc-solaris2.12 and x86_64-pc-linux-gnu. Ok for mainline? Ok. I think that addresses most all known issues. I'll pre-appove any additional targets people find as trivial. For example, if darwin10 doesn't pass, then *-*-darwin10* would be fine to add if that fixes the issue. I don't happen to have one that old to just test on.
On Tue, Feb 07, 2017 at 08:01:44AM -0800, Mike Stump wrote: > On Feb 7, 2017, at 2:20 AM, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote: > > No. In fact, I'd go for something like this: > > > > 2017-02-07 Dominik Vogt <vogt@linux.vnet.ibm.com> > > Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> > > > > * g++.dg/tls/thread_local-order2.C: Only xfail execution on > > *-*-solaris*. > > > > # HG changeset patch > > # Parent 031bb7a327cc984d387a8ae64e7c65d4b8793731 > > Only xfail g++.dg/tls/thread_local-order2.C on Solaris > > > > diff --git a/gcc/testsuite/g++.dg/tls/thread_local-order2.C b/gcc/testsuite/g++.dg/tls/thread_local-order2.C > > --- a/gcc/testsuite/g++.dg/tls/thread_local-order2.C > > +++ b/gcc/testsuite/g++.dg/tls/thread_local-order2.C > > @@ -2,10 +2,11 @@ > > // that isn't reverse order of construction. We need to move > > // __cxa_thread_atexit into glibc to get this right. > > > > -// { dg-do run { xfail *-*-* } } > > +// { dg-do run } > > // { dg-require-effective-target c++11 } > > // { dg-add-options tls } > > // { dg-require-effective-target tls_runtime } > > +// { dg-xfail-run-if "" { *-*-solaris* } } > > > > extern "C" void abort(); > > extern "C" int printf (const char *, ...); > > > > This way one can easily add per-target PR references or explanations, > > e.g. for darwin10 or others should they come up. > > > > Tested on i386-pc-solaris2.12 and x86_64-pc-linux-gnu. Ok for mainline? > > Ok. > > I think that addresses most all known issues. I'll pre-appove > any additional targets people find as trivial. For example, if > darwin10 doesn't pass, then *-*-darwin10* would be fine to add > if that fixes the issue. I don't happen to have one that old to > just test on. Here's a case of the test failing now: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79427 Powerpc64 BE with glibc-2.17 (2.18 reported to work). I'd be inclined to reply "upgrade Glibc to get rid of the FAIL" as that is what the test is supposed to find after all. What do you think? Ciao Dominik ^_^ ^_^
Hi Dominik, >> I think that addresses most all known issues. I'll pre-appove >> any additional targets people find as trivial. For example, if >> darwin10 doesn't pass, then *-*-darwin10* would be fine to add >> if that fixes the issue. I don't happen to have one that old to >> just test on. > > Here's a case of the test failing now: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79427 > > Powerpc64 BE with glibc-2.17 (2.18 reported to work). I'd be > inclined to reply "upgrade Glibc to get rid of the FAIL" as that > is what the test is supposed to find after all. What do you > think? agreed, that's what I usually do myself in similar situations. Unless you can easily check for the difference (like check for the presence of a function or facility), I let the test PASS on the newer version and live with the failure on older ones. The strange thing is that the test also passes on targets like darwin > 10 or AIX which certainly don't have __cxa_thread_atexit in libc. For some reason, the fallback implementation in libstdc++/libsupc++ isn't enough... Rainer
On Feb 9, 2017, at 1:31 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote: > > Here's a case of the test failing now: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79427 > > Powerpc64 BE with glibc-2.17 (2.18 reported to work). I'd be > inclined to reply "upgrade Glibc to get rid of the FAIL" as that > is what the test is supposed to find after all. What do you > think? The config triplet seems to be wonderfully devoid of information. :-( They next way to do it would be to copy the <features.h> style test case smell out a power64 BE 2.17 system and then avoid such a system, if one wanted to make it prettier. Otherwise, just note in the PR this is a known bug in glibc 2.17 and then mark as WONTFIX. It'd leave it up to the target folks if they want to spend the energy on making it pretty or not. On darwin, we manage this, but having os releases mixed into the triplet name, and then we can always say darwin10* won't work. I just ran config.guess on my ubuntu box, and appears to the the same string is was a decade ago. The feature test would appear to be something like: #include <features.h> #if (__GLIBC__ < 1) || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 18) die die die #endif if someone wants to make one.
Hi Mike, > On Feb 9, 2017, at 1:31 AM, Dominik Vogt <vogt@linux.vnet.ibm.com> wrote: >> >> Here's a case of the test failing now: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79427 >> >> Powerpc64 BE with glibc-2.17 (2.18 reported to work). I'd be >> inclined to reply "upgrade Glibc to get rid of the FAIL" as that >> is what the test is supposed to find after all. What do you >> think? > > The config triplet seems to be wonderfully devoid of information. :-( > > They next way to do it would be to copy the <features.h> style test case > smell out a power64 BE 2.17 system and then avoid such a system, if one > wanted to make it prettier. Otherwise, just note in the PR this is a known > bug in glibc 2.17 and then mark as WONTFIX. It'd leave it up to the target > folks if they want to spend the energy on making it pretty or not. On > darwin, we manage this, but having os releases mixed into the triplet name, > and then we can always say darwin10* won't work. I just ran config.guess > on my ubuntu box, and appears to the the same string is was a decade ago. > > The feature test would appear to be something like: > > #include <features.h> > #if (__GLIBC__ < 1) || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 18) > die die die > #endif > > if someone wants to make one. I'd rather not go this route if it can be avoided: it's completely contrary to the spirit of feature tests. If necessary, test for __cxa_thread_atexit in libc using check_function_available, although this seems only to apply to linux somehow. Rainer
# HG changeset patch # Parent 031bb7a327cc984d387a8ae64e7c65d4b8793731 Only xfail g++.dg/tls/thread_local-order2.C on Solaris diff --git a/gcc/testsuite/g++.dg/tls/thread_local-order2.C b/gcc/testsuite/g++.dg/tls/thread_local-order2.C --- a/gcc/testsuite/g++.dg/tls/thread_local-order2.C +++ b/gcc/testsuite/g++.dg/tls/thread_local-order2.C @@ -2,10 +2,11 @@ // that isn't reverse order of construction. We need to move // __cxa_thread_atexit into glibc to get this right. -// { dg-do run { xfail *-*-* } } +// { dg-do run } // { dg-require-effective-target c++11 } // { dg-add-options tls } // { dg-require-effective-target tls_runtime } +// { dg-xfail-run-if "" { *-*-solaris* } } extern "C" void abort(); extern "C" int printf (const char *, ...);