| Message ID | 11095502.M7ODFCTl7m@hardin.shanghai.arm.com |
|---|---|
| State | New |
| Headers | show |
Hi Thomas, On 05/01/16 07:37, Thomas Preud'homme wrote: > Hi, > > g++.dg/pr67989.C passes -march=armv4t to gcc when compiling which fails if > RUNTESTFLAGS passes -mcpu or -march with a different value. This patch adds a > dg-skip-if directive to skip the test when such a thing happens. > > ChangeLog entry is as follows: > > > *** gcc/testsuite/ChangeLog *** > > 2015-12-31 Thomas Preud'homme <thomas.preudhomme@arm.com> > > * g++.dg/pr67989.C: Skip test if already running it with -mcpu or > -march with different value. > > > diff --git a/gcc/testsuite/g++.dg/pr67989.C b/gcc/testsuite/g++.dg/pr67989.C > index > 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf825e7dc7 > 100644 > --- a/gcc/testsuite/g++.dg/pr67989.C > +++ b/gcc/testsuite/g++.dg/pr67989.C > @@ -1,5 +1,6 @@ > /* { dg-do compile } */ > /* { dg-options "-std=c++11 -O2" } */ > +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" "-mcpu=*" } > { "-march=armv4t" } } */ > /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } } */ > How about we try to do it using the add_options_for_arm_arch_v4t machinery and the arm_arch_v4t_ok check? I think the -marm part can go and can be added implicitly as part of multilib testing Thanks, Kyrill > __extension__ typedef unsigned long long int uint64_t; > > > Is this ok for stage3? > > Best regards, > > Thomas >
On 05/01/16 10:47, Kyrill Tkachov wrote: > Hi Thomas, > > On 05/01/16 07:37, Thomas Preud'homme wrote: >> Hi, >> >> g++.dg/pr67989.C passes -march=armv4t to gcc when compiling which >> fails if >> RUNTESTFLAGS passes -mcpu or -march with a different value. This patch >> adds a >> dg-skip-if directive to skip the test when such a thing happens. >> >> ChangeLog entry is as follows: >> >> >> *** gcc/testsuite/ChangeLog *** >> >> 2015-12-31 Thomas Preud'homme <thomas.preudhomme@arm.com> >> >> * g++.dg/pr67989.C: Skip test if already running it with >> -mcpu or >> -march with different value. >> >> >> diff --git a/gcc/testsuite/g++.dg/pr67989.C >> b/gcc/testsuite/g++.dg/pr67989.C >> index >> 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf825e7dc7 >> >> 100644 >> --- a/gcc/testsuite/g++.dg/pr67989.C >> +++ b/gcc/testsuite/g++.dg/pr67989.C >> @@ -1,5 +1,6 @@ >> /* { dg-do compile } */ >> /* { dg-options "-std=c++11 -O2" } */ >> +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" >> "-mcpu=*" } >> { "-march=armv4t" } } */ >> /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } >> } */ >> > > How about we try to do it using the add_options_for_arm_arch_v4t machinery > and the arm_arch_v4t_ok check? > > I think the -marm part can go and can be added implicitly as part of > multilib testing > Or we could drop all the target-specific options as this is supposed to be a generic test. Yes, I realise this was the particular flag combination required to trigger the original ICE, but no other target running this test has target-specific options, so it seams a little strange that ARM does. R. > Thanks, > Kyrill > > >> __extension__ typedef unsigned long long int uint64_t; >> >> >> Is this ok for stage3? >> >> Best regards, >> >> Thomas >> >
On 05/01/16 10:52, Richard Earnshaw (lists) wrote: > On 05/01/16 10:47, Kyrill Tkachov wrote: >> Hi Thomas, >> >> On 05/01/16 07:37, Thomas Preud'homme wrote: >>> Hi, >>> >>> g++.dg/pr67989.C passes -march=armv4t to gcc when compiling which >>> fails if >>> RUNTESTFLAGS passes -mcpu or -march with a different value. This patch >>> adds a >>> dg-skip-if directive to skip the test when such a thing happens. >>> >>> ChangeLog entry is as follows: >>> >>> >>> *** gcc/testsuite/ChangeLog *** >>> >>> 2015-12-31 Thomas Preud'homme <thomas.preudhomme@arm.com> >>> >>> * g++.dg/pr67989.C: Skip test if already running it with >>> -mcpu or >>> -march with different value. >>> >>> >>> diff --git a/gcc/testsuite/g++.dg/pr67989.C >>> b/gcc/testsuite/g++.dg/pr67989.C >>> index >>> 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf825e7dc7 >>> >>> 100644 >>> --- a/gcc/testsuite/g++.dg/pr67989.C >>> +++ b/gcc/testsuite/g++.dg/pr67989.C >>> @@ -1,5 +1,6 @@ >>> /* { dg-do compile } */ >>> /* { dg-options "-std=c++11 -O2" } */ >>> +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" >>> "-mcpu=*" } >>> { "-march=armv4t" } } */ >>> /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } >>> } */ >>> >> How about we try to do it using the add_options_for_arm_arch_v4t machinery >> and the arm_arch_v4t_ok check? >> >> I think the -marm part can go and can be added implicitly as part of >> multilib testing >> > Or we could drop all the target-specific options as this is supposed to > be a generic test. Yes, I realise this was the particular flag > combination required to trigger the original ICE, but no other target > running this test has target-specific options, so it seams a little > strange that ARM does. IIRC the problem in this PR was a fallback path in one of the atomic operation expand routines, so it needs an architecture version that is sufficiently low to not use the target-specific expanders. That's why the armv4t was there. Kyrill > R. > > >> Thanks, >> Kyrill >> >> >>> __extension__ typedef unsigned long long int uint64_t; >>> >>> >>> Is this ok for stage3? >>> >>> Best regards, >>> >>> Thomas >>>
On Tuesday, January 05, 2016 10:47:38 AM Kyrill Tkachov wrote: > Hi Thomas, Hi Kyrill, > > > > diff --git a/gcc/testsuite/g++.dg/pr67989.C > > b/gcc/testsuite/g++.dg/pr67989.C index > > 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf > > 825e7dc7 100644 > > --- a/gcc/testsuite/g++.dg/pr67989.C > > +++ b/gcc/testsuite/g++.dg/pr67989.C > > @@ -1,5 +1,6 @@ > > > > /* { dg-do compile } */ > > /* { dg-options "-std=c++11 -O2" } */ > > > > +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" > > "-mcpu=*" } { "-march=armv4t" } } */ > > > > /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } } > > */ > > How about we try to do it using the add_options_for_arm_arch_v4t machinery > and the arm_arch_v4t_ok check? I don't quite understand. dg-add-options doesn't take a selector according to GCC internals documentation and dg-additional-options doesn't take feature. If I use dg-add-options with a require-effective-target that will limit this test to ARM. Did I misunderstand your point? Best regards, Thomas
On 07/01/16 07:34, Thomas Preud'homme wrote: > On Tuesday, January 05, 2016 10:47:38 AM Kyrill Tkachov wrote: >> Hi Thomas, > Hi Kyrill, > >>> diff --git a/gcc/testsuite/g++.dg/pr67989.C >>> b/gcc/testsuite/g++.dg/pr67989.C index >>> 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf >>> 825e7dc7 100644 >>> --- a/gcc/testsuite/g++.dg/pr67989.C >>> +++ b/gcc/testsuite/g++.dg/pr67989.C >>> @@ -1,5 +1,6 @@ >>> >>> /* { dg-do compile } */ >>> /* { dg-options "-std=c++11 -O2" } */ >>> >>> +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" >>> "-mcpu=*" } { "-march=armv4t" } } */ >>> >>> /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } } >>> */ >> How about we try to do it using the add_options_for_arm_arch_v4t machinery >> and the arm_arch_v4t_ok check? > I don't quite understand. dg-add-options doesn't take a selector according to > GCC internals documentation and dg-additional-options doesn't take feature. If > I use dg-add-options with a require-effective-target that will limit this test > to ARM. > > Did I misunderstand your point? Humph, you're right. I thought that dg-add-options could take a target selector. In this case perhaps we should go the route of just removing the target-specific option altogether. Richard, that's the approach you recommended, right? Thanks, Kyrill > Best regards, > > Thomas
On 07/01/16 09:15, Kyrill Tkachov wrote: > > On 07/01/16 07:34, Thomas Preud'homme wrote: >> On Tuesday, January 05, 2016 10:47:38 AM Kyrill Tkachov wrote: >>> Hi Thomas, >> Hi Kyrill, >> >>>> diff --git a/gcc/testsuite/g++.dg/pr67989.C >>>> b/gcc/testsuite/g++.dg/pr67989.C index >>>> 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf >>>> >>>> 825e7dc7 100644 >>>> --- a/gcc/testsuite/g++.dg/pr67989.C >>>> +++ b/gcc/testsuite/g++.dg/pr67989.C >>>> @@ -1,5 +1,6 @@ >>>> >>>> /* { dg-do compile } */ >>>> /* { dg-options "-std=c++11 -O2" } */ >>>> >>>> +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" >>>> "-mcpu=*" } { "-march=armv4t" } } */ >>>> >>>> /* { dg-additional-options "-marm -march=armv4t" { target >>>> arm*-*-* } } >>>> */ >>> How about we try to do it using the add_options_for_arm_arch_v4t >>> machinery >>> and the arm_arch_v4t_ok check? >> I don't quite understand. dg-add-options doesn't take a selector >> according to >> GCC internals documentation and dg-additional-options doesn't take >> feature. If >> I use dg-add-options with a require-effective-target that will limit >> this test >> to ARM. >> >> Did I misunderstand your point? > > Humph, you're right. I thought that dg-add-options could take a target > selector. > In this case perhaps we should go the route of just removing the > target-specific option > altogether. > > Richard, that's the approach you recommended, right? > Yes. I think if you really need to test a specific set of target flags, then it might be acceptable to have a duplicate of the test in dg.target/arm (but please put a comment in the (arm version of the) test to explain why it has been duplicated. R. > Thanks, > Kyrill > >> Best regards, >> >> Thomas >
diff --git a/gcc/testsuite/g++.dg/pr67989.C b/gcc/testsuite/g++.dg/pr67989.C index 90261c450b4b9429fb989f7df62f3743017c7363..61be8e172a96df5bb76f7ecd8543dadf825e7dc7 100644 --- a/gcc/testsuite/g++.dg/pr67989.C +++ b/gcc/testsuite/g++.dg/pr67989.C @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-std=c++11 -O2" } */ +/* { dg-skip-if "do not override -mcpu" { arm*-*-* } { "-march=*" "-mcpu=*" } { "-march=armv4t" } } */ /* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } } */
Hi, g++.dg/pr67989.C passes -march=armv4t to gcc when compiling which fails if RUNTESTFLAGS passes -mcpu or -march with a different value. This patch adds a dg-skip-if directive to skip the test when such a thing happens. ChangeLog entry is as follows: *** gcc/testsuite/ChangeLog *** 2015-12-31 Thomas Preud'homme <thomas.preudhomme@arm.com> * g++.dg/pr67989.C: Skip test if already running it with -mcpu or -march with different value. __extension__ typedef unsigned long long int uint64_t; Is this ok for stage3? Best regards, Thomas