Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Mike Stump]

On Jan 26, 2016, at 10:35 PM, Thomas Preud'homme 
 wrote:
> On Monday, January 18, 2016 11:33:47 AM Thomas Preud'homme wrote:
>> On Wednesday, January 13, 2016 06:39:20 PM Bernd Schmidt wrote:
>>> On 01/12/2016 08:55 AM, Thomas Preud'homme wrote:
 On Monday, January 11, 2016 04:57:18 PM Bernd Schmidt wrote:
> On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:
>> 2016-01-08  Thomas Preud'homme  
>> 
>>  * g++.dg/pr67989.C: Remove ARM-specific option.
>>  * gcc.target/arm/pr67989.C: New file.
> 
> I checked some other arm tests and they have things like
> 
> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> "-march=*" } { "-march=armv4t" } } */
> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> "-mthumb" } { "" } } */
> 
> Do you need the same in your testcase?
 
 That was the first approach I took but Kyrill suggested me to use
 arm_arch_v4t and arm_arch_v4t_ok machinery instead. It should take care
 about whether the architecture can be selected.
>>> 
>>> Hmm, the ones I looked at did use dg-add-options, but not the
>>> corresponding _ok requirement. So I think this is OK.
>> 
>> Just to make sure: ok as in OK to commit as is?

Ok.

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

Ping?

On Monday, January 18, 2016 11:33:47 AM Thomas Preud'homme wrote:
> On Wednesday, January 13, 2016 06:39:20 PM Bernd Schmidt wrote:
> > On 01/12/2016 08:55 AM, Thomas Preud'homme wrote:
> > > On Monday, January 11, 2016 04:57:18 PM Bernd Schmidt wrote:
> > >> On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:
> > >>> 2016-01-08  Thomas Preud'homme  
> > >>> 
> > >>>   * g++.dg/pr67989.C: Remove ARM-specific option.
> > >>>   * gcc.target/arm/pr67989.C: New file.
> > >> 
> > >> I checked some other arm tests and they have things like
> > >> 
> > >> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> > >> "-march=*" } { "-march=armv4t" } } */
> > >> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> > >> "-mthumb" } { "" } } */
> > >> 
> > >> Do you need the same in your testcase?
> > > 
> > > That was the first approach I took but Kyrill suggested me to use
> > > arm_arch_v4t and arm_arch_v4t_ok machinery instead. It should take care
> > > about whether the architecture can be selected.
> > 
> > Hmm, the ones I looked at did use dg-add-options, but not the
> > corresponding _ok requirement. So I think this is OK.
> 
> Just to make sure: ok as in OK to commit as is?
> 
> Best regards,
> 
> Thomas

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

On Wednesday, January 13, 2016 06:39:20 PM Bernd Schmidt wrote:
> On 01/12/2016 08:55 AM, Thomas Preud'homme wrote:
> > On Monday, January 11, 2016 04:57:18 PM Bernd Schmidt wrote:
> >> On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:
> >>> 2016-01-08  Thomas Preud'homme  
> >>> 
> >>>   * g++.dg/pr67989.C: Remove ARM-specific option.
> >>>   * gcc.target/arm/pr67989.C: New file.
> >> 
> >> I checked some other arm tests and they have things like
> >> 
> >> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> >> "-march=*" } { "-march=armv4t" } } */
> >> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> >> "-mthumb" } { "" } } */
> >> 
> >> Do you need the same in your testcase?
> > 
> > That was the first approach I took but Kyrill suggested me to use
> > arm_arch_v4t and arm_arch_v4t_ok machinery instead. It should take care
> > about whether the architecture can be selected.
> 
> Hmm, the ones I looked at did use dg-add-options, but not the
> corresponding _ok requirement. So I think this is OK.

Just to make sure: ok as in OK to commit as is?

Best regards,

Thomas

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Bernd Schmidt]

On 01/12/2016 08:55 AM, Thomas Preud'homme wrote:

On Monday, January 11, 2016 04:57:18 PM Bernd Schmidt wrote:

On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:

2016-01-08  Thomas Preud'homme  

  * g++.dg/pr67989.C: Remove ARM-specific option.
  * gcc.target/arm/pr67989.C: New file.


I checked some other arm tests and they have things like

/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
"-march=*" } { "-march=armv4t" } } */
/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
"-mthumb" } { "" } } */

Do you need the same in your testcase?


That was the first approach I took but Kyrill suggested me to use arm_arch_v4t
and arm_arch_v4t_ok machinery instead. It should take care about whether the
architecture can be selected.


Hmm, the ones I looked at did use dg-add-options, but not the 
corresponding _ok requirement. So I think this is OK.



Bernd

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

On Monday, January 11, 2016 04:57:18 PM Bernd Schmidt wrote:
> On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:
> > 2016-01-08  Thomas Preud'homme  
> > 
> >  * g++.dg/pr67989.C: Remove ARM-specific option.
> >  * gcc.target/arm/pr67989.C: New file.
> 
> I checked some other arm tests and they have things like
> 
> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> "-march=*" } { "-march=armv4t" } } */
> /* { dg-skip-if "avoid conflicting multilib options" { *-*-* } {
> "-mthumb" } { "" } } */
> 
> Do you need the same in your testcase?

That was the first approach I took but Kyrill suggested me to use arm_arch_v4t 
and arm_arch_v4t_ok machinery instead. It should take care about whether the 
architecture can be selected.

Best regards,

Thomas

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Bernd Schmidt]

On 01/08/2016 10:33 AM, Thomas Preud'homme wrote:

2016-01-08  Thomas Preud'homme  

 * g++.dg/pr67989.C: Remove ARM-specific option.
 * gcc.target/arm/pr67989.C: New file.


I checked some other arm tests and they have things like

/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { 
"-march=*" } { "-march=armv4t" } } */
/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { 
"-mthumb" } { "" } } */


Do you need the same in your testcase?


Bernd

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

On Thursday, January 07, 2016 10:26:28 AM Richard Earnshaw wrote:
> On 07/01/16 09:15, Kyrill Tkachov wrote:
> > 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.

What about the following:


*** gcc/testsuite/ChangeLog ***

2016-01-08  Thomas Preud'homme  

* g++.dg/pr67989.C: Remove ARM-specific option.
* gcc.target/arm/pr67989.C: New file.


diff --git a/gcc/testsuite/g++.dg/pr67989.C b/gcc/testsuite/g++.dg/pr67989.C
index 
90261c450b4b9429fb989f7df62f3743017c7363..c3023557d31a21aead717fd58483c82e3e74da95
 
100644
--- a/gcc/testsuite/g++.dg/pr67989.C
+++ b/gcc/testsuite/g++.dg/pr67989.C
@@ -1,6 +1,5 @@
 /* { dg-do compile } */
 /* { dg-options "-std=c++11 -O2" } */
-/* { dg-additional-options "-marm -march=armv4t" { target arm*-*-* } } */
 
 __extension__ typedef unsigned long long int uint64_t;
 namespace std __attribute__ ((__visibility__ ("default")))
diff --git a/gcc/testsuite/gcc.target/arm/pr67989.C b/gcc/testsuite/
gcc.target/arm/pr67989.C
new file mode 100644
index 
..0006924e24f698711e1e501d09b5098049522ad6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr67989.C
@@ -0,0 +1,82 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c++11 -O2" } */
+/* { dg-require-effective-target arm_arch_v4t_ok } */
+/* { dg-add-options arm_arch_v4t } */
+/* { dg-additional-options "-marm" } */
+
+/* Duplicate version of the test in g++.dg to be able to run this test only 
if
+   ARMv4t in ARM execution state can be targetted.  Newer architecture don't
+   expose the bug this testcase was written for.  */
+
+
+__extension__ typedef unsigned long long int uint64_t;
+namespace std __attribute__ ((__visibility__ ("default")))
+{
+  typedef enum memory_order
+  {
+memory_order_seq_cst
+  } memory_order;
+}
+
+namespace std __attribute__ ((__visibility__ ("default")))
+{
+  template < typename _Tp > struct atomic
+  {
+static constexpr int _S_min_alignment
+  = (sizeof (_Tp) & (sizeof (_Tp) - 1)) || sizeof (_Tp) > 16
+  ? 0 : sizeof (_Tp);
+static constexpr int _S_alignment
+  = _S_min_alignment > alignof (_Tp) ? _S_min_alignment : alignof (_Tp);
+  alignas (_S_alignment) _Tp _M_i;
+operator  _Tp () const noexcept
+{
+  return load ();
+}
+_Tp load (memory_order __m = memory_order_seq_cst) const noexcept
+{
+  _Tp tmp;
+__atomic_load (&_M_i, , __m);
+}
+  };
+}
+
+namespace lldb_private
+{
+  namespace imp
+  {
+  }
+  class Address;
+}
+namespace lldb
+{
+  typedef uint64_t addr_t;
+  class SBSection
+  {
+  };
+  class SBAddress
+  {
+void SetAddress (lldb::SBSection section, lldb::addr_t offset);
+  lldb_private::Address & ref ();
+  };
+}
+namespace lldb_private
+{
+  class Address
+  {
+  public:
+const Address & SetOffset (lldb::addr_t offset)
+{
+  bool changed = m_offset != offset;
+}
+std::atomic < lldb::addr_t > m_offset;
+  };
+}
+
+using namespace lldb;
+using namespace lldb_private;
+void
+SBAddress::SetAddress (lldb::SBSection section, lldb::addr_t offset)
+{
+  Address & addr = ref ();
+  addr.SetOffset (offset);
+}


Is this ok for stage3?

Best regards,

Thomas

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Richard Earnshaw (lists)]

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
> 

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

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

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Richard Earnshaw (lists)]

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  
>>
>>  * 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
>>
> 

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Kyrill Tkachov]

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  

 * 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

Re: [PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Kyrill Tkachov]

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  

  * 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

[PATCH, testsuite] Fix g++.dg/pr67989.C test failure when running with -march or -mcpu

[Thomas Preud'homme]

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  

* 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*-*-* } } */
 
 __extension__ typedef unsigned long long int uint64_t;


Is this ok for stage3?

Best regards,

Thomas