public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu=
@ 2016-12-01 11:54 Georg-Johann Lay
  2016-12-01 16:40 ` Mike Stump
  0 siblings, 1 reply; 5+ messages in thread
From: Georg-Johann Lay @ 2016-12-01 11:54 UTC (permalink / raw)
  To: gcc-patches
  Cc: Denis Chertykov, Senthil Kumar Selvaraj, Pitchumani Sivanupandi

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

This patch moves the compile tests that have a hard coded -mmcu=MCU in 
their dg-options to a new folder.

The exp driver filters out -mmcu= from the command line options that are 
provided by, say, board description files or --tool-opts.

This is needed because otherwise conflicting -mmcu= will FAIL respective 
test cases because of "specified option '-mmcu' more than once" errors 
from avr-gcc.

Ok for trunk?

Johann

gcc/testsuite/
	* gcc.target/avr/mmcu: New folder for compile-tests with -mmcu=.
	* gcc.target/avr/mmcu/avr-mmcu.exp: New file.
	* gcc.target/avr/pr58545.c: Move to gcc.target/avr/mmcu.
	* gcc.target/avr/tiny-caller-save.c: Dito.
	* gcc.target/avr/tiny-memx.c: Dito.

[-- Attachment #2: testsuite-avr-mmcu-exp.diff --]
[-- Type: text/x-patch, Size: 7469 bytes --]

Index: gcc.target/avr/mmcu/avr-mmcu.exp
===================================================================
--- gcc.target/avr/mmcu/avr-mmcu.exp	(nonexistent)
+++ gcc.target/avr/mmcu/avr-mmcu.exp	(working copy)
@@ -0,0 +1,99 @@
+# Copyright (C) 2008-2016 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+# 
+# You should have received a copy of the GNU General Public License
+# along with GCC; see the file COPYING3.  If not see
+# <http://www.gnu.org/licenses/>.
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# This folder contains compile tests that set dg-options to
+# some -mmcu=<MCU> which might collide with the MCU set by the
+# target board.  This in turn will fail the test case due to
+# "error: specified option '-mmcu' more than once".
+#
+# Hence we filter out -mmcu= from cflags and --tool_opts before
+# running the tests.
+
+# Exit immediately if this isn't an AVR target.
+if ![istarget avr-*-*] then {
+  return
+}
+
+# Return the saved values of the variable_list
+proc save_variables { variable_list } {
+    set saved_variable { }
+
+    foreach variable $variable_list {
+	upvar 1 $variable  var
+
+	set save($variable) $var
+	lappend saved_variable $save($variable)
+    }
+    return $saved_variable
+}
+
+# Restore the values of the variable_list
+proc restore_variables { variable_list saved_variable } {
+    foreach variable $variable_list value $saved_variable {
+	upvar 1 $variable  var
+	set var $value
+    }
+}
+
+# Filter out -mmcu= options
+proc filter_out_mmcu { options } {
+    set reduced {}
+    
+    foreach option [ split $options ] {
+	if { ![ regexp "\-mmcu=.*" $option ] } {
+	    lappend reduced $option
+	}
+    }
+
+    return [ join $reduced " " ]
+}
+
+# Load support procs.
+load_lib gcc-dg.exp
+
+# If a testcase doesn't have special options, use these.
+global DEFAULT_CFLAGS
+if ![info exists DEFAULT_CFLAGS] then {
+    set DEFAULT_CFLAGS " -ansi -pedantic-errors"
+}
+
+# If no --tool_opts were specified, use empty ones.
+if ![info exists TOOL_OPTIONS] then {
+    set TOOL_OPTIONS ""
+}
+
+# Initialize `dg'.
+dg-init
+
+# Save
+set variablelist [ list TOOL_OPTIONS board_info([target_info name],cflags) ]
+set saved_value [ save_variables $variablelist ]
+
+# Filter-out -mmcu=
+set TOOL_OPTIONS [ filter_out_mmcu $TOOL_OPTIONS ]
+set board_info([ target_info name ],cflags) [ filter_out_mmcu $board_info([ target_info name ],cflags) ] 
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.{\[cCS\],cpp}]] \
+	"" $DEFAULT_CFLAGS
+
+# Restore
+restore_variables $variablelist $saved_value
+
+# All done.
+dg-finish
Index: gcc.target/avr/mmcu/pr58545.c
===================================================================
Index: gcc.target/avr/mmcu/tiny-caller-save.c
===================================================================
--- gcc.target/avr/mmcu/tiny-caller-save.c	(revision 243105)
+++ gcc.target/avr/mmcu/tiny-caller-save.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile { target avr_tiny } } */
+/* { dg-do compile } */
 /* { dg-options "-mmcu=avrtiny -gdwarf -Os" } */
 
 /* This is a stripped down piece of libgcc2.c that triggerd an ICE for avr with
Index: gcc.target/avr/mmcu/tiny-memx.c
===================================================================
--- gcc.target/avr/mmcu/tiny-memx.c	(revision 243105)
+++ gcc.target/avr/mmcu/tiny-memx.c	(working copy)
@@ -1,4 +1,4 @@
-/* { dg-do compile { target avr_tiny } } */
+/* { dg-do compile } */
 /* { dg-options "-mmcu=avrtiny" } */
 
 const __memx char ascmonth[] = "Jan"; /* { dg-error "not supported" } */
Index: gcc.target/avr/pr58545.c
===================================================================
--- gcc.target/avr/pr58545.c	(revision 243104)
+++ gcc.target/avr/pr58545.c	(nonexistent)
@@ -1,23 +0,0 @@
-/* { dg-do compile } */
-/* { dg-options "-Os -mmcu=atmega8" } */
-
-typedef unsigned char uint8_t;
-typedef unsigned int uint16_t;
-
-extern uint8_t f1 (const uint8_t*);
-extern void f2 (uint8_t*, uint8_t);
-
-void func (uint16_t parameter, uint8_t *addr, uint8_t data)
-{
-   uint8_t status;
-
-   status = f1 (addr + 8);
-
-   addr++;
-
-   if (*addr == parameter + 8)
-      *addr = parameter;
-
-   f2 (addr, data);
-   f2 (addr + 8, status + 1);
-}
Index: gcc.target/avr/tiny-caller-save.c
===================================================================
--- gcc.target/avr/tiny-caller-save.c	(revision 243105)
+++ gcc.target/avr/tiny-caller-save.c	(nonexistent)
@@ -1,78 +0,0 @@
-/* { dg-do compile { target avr_tiny } } */
-/* { dg-options "-mmcu=avrtiny -gdwarf -Os" } */
-
-/* This is a stripped down piece of libgcc2.c that triggerd an ICE for avr with
-   "-mmcu=avrtiny -g -Os"; replace_reg_with_saved_mem would generate:
-   (concatn:SI [
-                    (reg:SI 18 r18)
-                    (reg:SI 19 r19)
-                    (mem/c:QI (plus:HI (reg/f:HI 28 r28)
-                            (const_int 43 [0x2b])) [6  S1 A8])
-                    (mem/c:QI (plus:HI (reg/f:HI 28 r28)
-                            (const_int 44 [0x2c])) [6  S1 A8])
-                ]) */
-
-typedef int SItype __attribute__ ((mode (SI)));
-typedef unsigned int USItype __attribute__ ((mode (SI)));
-typedef int DItype __attribute__ ((mode (DI)));
-typedef unsigned int UDItype __attribute__ ((mode (DI)));
-struct DWstruct
-{
-  SItype low, high;
-};
-typedef union
-{
-  struct DWstruct s;
-  DItype ll;
-} DWunion;
-
-UDItype
-__udivmoddi4 (UDItype n, UDItype d)
-{
-  const DWunion nn = {.ll = n };
-  const DWunion dd = {.ll = d };
-  USItype d0, d1, n2;
-  USItype q0;
-
-  d0 = dd.s.low;
-  d1 = dd.s.high;
-  n2 = nn.s.high;
-
-      USItype m0;
-
-      do
-	{
-	  USItype __d1, __d0, __q1, __q0;
-	  USItype __r1, __m;
-	  __d1 = ((USItype) (d1) >> 16);
-	  __d0 = ((USItype) (d1) & (((USItype) 1 << 16) - 1));
-	  __r1 = (n2) % __d1;
-	  __q1 = (n2) / __d1;
-	  __m = (USItype) __q1 *__d0;
-	  __r1 -= __m;
-	  __q0 = __r1 / __d1;
-	  (q0) = (USItype) __q1 *((USItype) 1 << 16) | __q0;
-	}
-      while (0);
-      do
-	{
-	  USItype __x0, __x1, __x2;
-	  USItype __ul, __vl, __uh, __vh;
-	  __ul = ((USItype) (q0) & (((USItype) 1 << 16) - 1));
-	  __uh = ((USItype) (q0) >> 16);
-	  __vl = ((USItype) (d0) & (((USItype) 1 << 16) - 1));
-	  __vh = ((USItype) (d0) >> 16);
-	  __x0 = (USItype) __ul *__vl;
-	  __x1 = (USItype) __ul *__vh;
-	  __x2 = (USItype) __uh *__vl;
-	  __x1 += ((USItype) (__x0) >> 16);
-	  __x1 += __x2;
-	  (m0) =
-	    ((USItype) (__x1) & (((USItype) 1 << 16) - 1)) *
-	    ((USItype) 1 << 16) +
-	    ((USItype) (__x0) & (((USItype) 1 << 16) - 1));
-	}
-      while (0);
-
-return m0;
-}
Index: gcc.target/avr/tiny-memx.c
===================================================================
--- gcc.target/avr/tiny-memx.c	(revision 243105)
+++ gcc.target/avr/tiny-memx.c	(nonexistent)
@@ -1,4 +0,0 @@
-/* { dg-do compile { target avr_tiny } } */
-/* { dg-options "-mmcu=avrtiny" } */
-
-const __memx char ascmonth[] = "Jan"; /* { dg-error "not supported" } */

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu=
  2016-12-01 11:54 [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu= Georg-Johann Lay
@ 2016-12-01 16:40 ` Mike Stump
  2016-12-02 10:21   ` Georg-Johann Lay
  0 siblings, 1 reply; 5+ messages in thread
From: Mike Stump @ 2016-12-01 16:40 UTC (permalink / raw)
  To: Georg-Johann Lay
  Cc: gcc-patches, Denis Chertykov, Senthil Kumar Selvaraj,
	Pitchumani Sivanupandi

On Dec 1, 2016, at 3:54 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
> 
> This patch moves the compile tests that have a hard coded -mmcu=MCU in their dg-options to a new folder.
> 
> The exp driver filters out -mmcu= from the command line options that are provided by, say, board description files or --tool-opts.
> 
> This is needed because otherwise conflicting -mmcu= will FAIL respective test cases because of "specified option '-mmcu' more than once" errors from avr-gcc.
> 
> Ok for trunk?

So, it would be nice if different ports can use roughly similar schemes to handle the same problems.  I think arm is one of the more complex ports at this point in this regard with a lot of people and a lot of years time to contemplate and implement solutions to the problem.  They in particular don't have to move test cases around to handle the difference like this, I think it would be best to avoid that requirement if possible.

Glancing around, two starting points for how the arm achieves what it does:

  lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts with -m(cpu|arch)=.* switch"

in arm.exp, and they use something like:

/* { dg-require-effective-target arm_crypto_ok } */                                                 |crypto-vsha256hq_u32.c:2:/* { dg-require-effective-target arm_crypto_ok } */
/* { dg-add-options arm_crypto } */                                                                 |crypto-vsha256su0q_u32.c:2:/* { dg-require-effective-target arm_crypto_ok } */

to validate the requirements of the test case, and to ensure that optional things are selected.  Nice, simple, extensible, handles multilibs, dejagnu arguments and different cpu defaults as I recall.

You won't need all the hair the arm folks have, but if you stub in support in that direction, you then have simple, easy expansion room to match all complexities that the arm folks have already hit and solved.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu=
  2016-12-01 16:40 ` Mike Stump
@ 2016-12-02 10:21   ` Georg-Johann Lay
  2016-12-13  9:37     ` Ping #1: " Georg-Johann Lay
  0 siblings, 1 reply; 5+ messages in thread
From: Georg-Johann Lay @ 2016-12-02 10:21 UTC (permalink / raw)
  To: Mike Stump
  Cc: gcc-patches, Denis Chertykov, Senthil Kumar Selvaraj,
	Pitchumani Sivanupandi

On 01.12.2016 17:40, Mike Stump wrote:
> On Dec 1, 2016, at 3:54 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>
>> This patch moves the compile tests that have a hard coded -mmcu=MCU in their dg-options to a new folder.
>>
>> The exp driver filters out -mmcu= from the command line options that are provided by, say, board description files or --tool-opts.
>>
>> This is needed because otherwise conflicting -mmcu= will FAIL respective test cases because of "specified option '-mmcu' more than once" errors from avr-gcc.
>>
>> Ok for trunk?
>
> So, it would be nice if different ports can use roughly similar schemes to handle the same problems.  I think arm is one of the more complex ports at this point in this regard with a lot of people and a lot of years time to contemplate and implement solutions to the problem.  They in particular don't have to move test cases around to handle the difference like this, I think it would be best to avoid that requirement if possible.
>
> Glancing around, two starting points for how the arm achieves what it does:
>
>   lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.* conflicts with -m(cpu|arch)=.* switch"
>
> in arm.exp, and they use something like:
>
> /* { dg-require-effective-target arm_crypto_ok } */                                                 |crypto-vsha256hq_u32.c:2:/* { dg-require-effective-target arm_crypto_ok } */
> /* { dg-add-options arm_crypto } */                                                                 |crypto-vsha256su0q_u32.c:2:/* { dg-require-effective-target arm_crypto_ok } */
>
> to validate the requirements of the test case, and to ensure that
> optional things are selected.  Nice, simple, extensible, handles
> multilibs, dejagnu arguments and different cpu defaults as I recall.
>
> You won't need all the hair the arm folks have, but if you stub in
> support in that direction, you then have simple, easy expansion room
> to match all complexities that the arm folks have already hit and
> solved.

I tried this approach, but it does not work as expected.

Notice that avr-gcc throws an error if conflicting -mmcu options are 
supplied.  Pruning the output will make some tests "PASS", but the 
compiler didn't actually do anything but producing an error message...

And one test FAILs because it should produce some specific diagnostic, 
but again the compiler just throws a different error, the output is 
pruned and the expected message is missing, hence fail.

Also one test case is for ATmega8, but you won't run the whole test 
suite against ATmega8 because that device is too restricted to give 
reasonable results...  Hence for some tests -mmcu=atmega8 is added by hand.

Johann




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Ping #1: [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu=
  2016-12-02 10:21   ` Georg-Johann Lay
@ 2016-12-13  9:37     ` Georg-Johann Lay
  2016-12-13 17:46       ` Denis Chertykov
  0 siblings, 1 reply; 5+ messages in thread
From: Georg-Johann Lay @ 2016-12-13  9:37 UTC (permalink / raw)
  To: Mike Stump
  Cc: gcc-patches, Denis Chertykov, Senthil Kumar Selvaraj,
	Pitchumani Sivanupandi

Ping #1

As I explained below, the solution taken be arm (pruning output)
does not work here.

Johann

On 02.12.2016 11:21, Georg-Johann Lay wrote:
> On 01.12.2016 17:40, Mike Stump wrote:
>> On Dec 1, 2016, at 3:54 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>
>>> This patch moves the compile tests that have a hard coded -mmcu=MCU
>>> in their dg-options to a new folder.
>>>
>>> The exp driver filters out -mmcu= from the command line options that
>>> are provided by, say, board description files or --tool-opts.
>>>
>>> This is needed because otherwise conflicting -mmcu= will FAIL
>>> respective test cases because of "specified option '-mmcu' more than
>>> once" errors from avr-gcc.
>>>
>>> Ok for trunk?
>>
>> So, it would be nice if different ports can use roughly similar
>> schemes to handle the same problems.  I think arm is one of the more
>> complex ports at this point in this regard with a lot of people and a
>> lot of years time to contemplate and implement solutions to the
>> problem.  They in particular don't have to move test cases around to
>> handle the difference like this, I think it would be best to avoid
>> that requirement if possible.
>>
>> Glancing around, two starting points for how the arm achieves what it
>> does:
>>
>>   lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.*
>> conflicts with -m(cpu|arch)=.* switch"
>>
>> in arm.exp, and they use something like:
>>
>> /* { dg-require-effective-target arm_crypto_ok }
>> */
>> |crypto-vsha256hq_u32.c:2:/* { dg-require-effective-target
>> arm_crypto_ok } */
>> /* { dg-add-options arm_crypto }
>> */
>> |crypto-vsha256su0q_u32.c:2:/* { dg-require-effective-target
>> arm_crypto_ok } */
>>
>> to validate the requirements of the test case, and to ensure that
>> optional things are selected.  Nice, simple, extensible, handles
>> multilibs, dejagnu arguments and different cpu defaults as I recall.
>>
>> You won't need all the hair the arm folks have, but if you stub in
>> support in that direction, you then have simple, easy expansion room
>> to match all complexities that the arm folks have already hit and
>> solved.
>
> I tried this approach, but it does not work as expected.
>
> Notice that avr-gcc throws an error if conflicting -mmcu options are
> supplied.  Pruning the output will make some tests "PASS", but the
> compiler didn't actually do anything but producing an error message...
>
> And one test FAILs because it should produce some specific diagnostic,
> but again the compiler just throws a different error, the output is
> pruned and the expected message is missing, hence fail.
>
> Also one test case is for ATmega8, but you won't run the whole test
> suite against ATmega8 because that device is too restricted to give
> reasonable results...  Hence for some tests -mmcu=atmega8 is added by hand.
>
> Johann
>
>
>
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Ping #1: [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu=
  2016-12-13  9:37     ` Ping #1: " Georg-Johann Lay
@ 2016-12-13 17:46       ` Denis Chertykov
  0 siblings, 0 replies; 5+ messages in thread
From: Denis Chertykov @ 2016-12-13 17:46 UTC (permalink / raw)
  To: Georg-Johann Lay
  Cc: Mike Stump, gcc-patches, Senthil Kumar Selvaraj, Pitchumani Sivanupandi

2016-12-13 13:36 GMT+04:00 Georg-Johann Lay <avr@gjlay.de>:
> Ping #1
>
> As I explained below, the solution taken be arm (pruning output)
> does not work here.

Approved.
Please apply your patch.

>
> Johann
>
> On 02.12.2016 11:21, Georg-Johann Lay wrote:
>>
>> On 01.12.2016 17:40, Mike Stump wrote:
>>>
>>> On Dec 1, 2016, at 3:54 AM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>>
>>>>
>>>> This patch moves the compile tests that have a hard coded -mmcu=MCU
>>>> in their dg-options to a new folder.
>>>>
>>>> The exp driver filters out -mmcu= from the command line options that
>>>> are provided by, say, board description files or --tool-opts.
>>>>
>>>> This is needed because otherwise conflicting -mmcu= will FAIL
>>>> respective test cases because of "specified option '-mmcu' more than
>>>> once" errors from avr-gcc.
>>>>
>>>> Ok for trunk?
>>>
>>>
>>> So, it would be nice if different ports can use roughly similar
>>> schemes to handle the same problems.  I think arm is one of the more
>>> complex ports at this point in this regard with a lot of people and a
>>> lot of years time to contemplate and implement solutions to the
>>> problem.  They in particular don't have to move test cases around to
>>> handle the difference like this, I think it would be best to avoid
>>> that requirement if possible.
>>>
>>> Glancing around, two starting points for how the arm achieves what it
>>> does:
>>>
>>>   lappend dg_runtest_extra_prunes "warning: switch -m(cpu|arch)=.*
>>> conflicts with -m(cpu|arch)=.* switch"
>>>
>>> in arm.exp, and they use something like:
>>>
>>> /* { dg-require-effective-target arm_crypto_ok }
>>> */
>>> |crypto-vsha256hq_u32.c:2:/* { dg-require-effective-target
>>> arm_crypto_ok } */
>>> /* { dg-add-options arm_crypto }
>>> */
>>> |crypto-vsha256su0q_u32.c:2:/* { dg-require-effective-target
>>> arm_crypto_ok } */
>>>
>>> to validate the requirements of the test case, and to ensure that
>>> optional things are selected.  Nice, simple, extensible, handles
>>> multilibs, dejagnu arguments and different cpu defaults as I recall.
>>>
>>> You won't need all the hair the arm folks have, but if you stub in
>>> support in that direction, you then have simple, easy expansion room
>>> to match all complexities that the arm folks have already hit and
>>> solved.
>>
>>
>> I tried this approach, but it does not work as expected.
>>
>> Notice that avr-gcc throws an error if conflicting -mmcu options are
>> supplied.  Pruning the output will make some tests "PASS", but the
>> compiler didn't actually do anything but producing an error message...
>>
>> And one test FAILs because it should produce some specific diagnostic,
>> but again the compiler just throws a different error, the output is
>> pruned and the expected message is missing, hence fail.
>>
>> Also one test case is for ATmega8, but you won't run the whole test
>> suite against ATmega8 because that device is too restricted to give
>> reasonable results...  Hence for some tests -mmcu=atmega8 is added by
>> hand.
>>
>> Johann
>>
>>
>>
>>
>>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-12-13 17:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01 11:54 [patch,testsuite,avr]: Filter-out -mmcu= from options for tests that set -mmcu= Georg-Johann Lay
2016-12-01 16:40 ` Mike Stump
2016-12-02 10:21   ` Georg-Johann Lay
2016-12-13  9:37     ` Ping #1: " Georg-Johann Lay
2016-12-13 17:46       ` Denis Chertykov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).