public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
@ 2011-09-01  1:40 Terry Guo
  2011-09-01  6:14 ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Terry Guo @ 2011-09-01  1:40 UTC (permalink / raw)
  To: gcc-patches; +Cc: ro, mikestump

Hello,

There are some bitfield definitions in this case, such as "enum E2 a:31".
For most targets, the ENUM type size equals INT type so this definition is
OK. But for some targets like arm_eabi, the default ENUM type size could be
the smallest integer type that can contain all of its enumerated values. For
this case, the size of variable a is 8 bits. Thus the above definition is
regarded as an error.

We could fix this issue in two methods: compile the cases with option
-fno-short-enums or just skip the case for targets with short enums. For the
first one, the user must also provide a library built with option
-fno-short-enums, otherwise linking them together could cause subtle issues.
This patch prefers the last method. If the no-short-enums library is ready,
user can still run this case by performing "make check" with extra option
-fno-short-enums.

Here is the patch, is it OK to commit?

Best regards,
Terry

2011-08-31  Terry Guo  <terry.guo@arm.com>

        * gcc.dg/compat/struct-layout-1_main.c: Skip the case
        if the target uses short enums.

diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
index b59453e..64275ea 100644
--- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
+++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
@@ -1,4 +1,5 @@
 /* { dg-prune-output ".*-Wno-abi.*" } */
+/* { dg-skip-if "" { short_enums } } */
  
   #include "struct-layout-1.h"


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

* Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  1:40 [Patch, testsuite] Skip case struct-layout-1 for targets using short enums Terry Guo
@ 2011-09-01  6:14 ` Jakub Jelinek
  2011-09-01  6:34   ` Terry Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2011-09-01  6:14 UTC (permalink / raw)
  To: Terry Guo; +Cc: gcc-patches, ro, mikestump

On Thu, Sep 01, 2011 at 09:34:20AM +0800, Terry Guo wrote:
> Here is the patch, is it OK to commit?

Definitely not.
struct-layout-1* already has support for short enums, see
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110247
So, if that doesn't work for you, you need to debug why.

> 2011-08-31  Terry Guo  <terry.guo@arm.com>
> 
>         * gcc.dg/compat/struct-layout-1_main.c: Skip the case
>         if the target uses short enums.
> 
> diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> index b59453e..64275ea 100644
> --- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> +++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> @@ -1,4 +1,5 @@
>  /* { dg-prune-output ".*-Wno-abi.*" } */
> +/* { dg-skip-if "" { short_enums } } */
>   
>    #include "struct-layout-1.h"
> 

	Jakub

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

* RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  6:14 ` Jakub Jelinek
@ 2011-09-01  6:34   ` Terry Guo
  2011-09-01  6:46     ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Terry Guo @ 2011-09-01  6:34 UTC (permalink / raw)
  To: 'Jakub Jelinek'; +Cc: gcc-patches, ro, mikestump

Hello Jakub,

> 
> On Thu, Sep 01, 2011 at 09:34:20AM +0800, Terry Guo wrote:
> > Here is the patch, is it OK to commit?
> 
> Definitely not.
> struct-layout-1* already has support for short enums, see
> http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=110247
> So, if that doesn't work for you, you need to debug why.

But it still fails for arm-none-eabi as you can see in test results at
http://gcc.gnu.org/ml/gcc-testresults/2011-08/msg02739.html. 

FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile
FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile
UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o
link 
UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o
execute

On my machine, the error message is "error: width of 'a' exceeds its type".
My GCC is cross built for arm-none-eabi based on upstream GCC 4.6. The
command I run this case is simply as: "make check-gcc
RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-m3
compat.exp=struct-layout-1*". Are there anything else I should do to pass
this case? 


> 
> > 2011-08-31  Terry Guo  <terry.guo@arm.com>
> >
> >         * gcc.dg/compat/struct-layout-1_main.c: Skip the case
> >         if the target uses short enums.
> >
> > diff --git a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> > b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> > index b59453e..64275ea 100644
> > --- a/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> > +++ b/gcc/testsuite/gcc.dg/compat/struct-layout-1_main.c
> > @@ -1,4 +1,5 @@
> >  /* { dg-prune-output ".*-Wno-abi.*" } */
> > +/* { dg-skip-if "" { short_enums } } */
> >
> >    #include "struct-layout-1.h"
> >
> 
> 	Jakub

Regards,
Terry



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

* Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  6:34   ` Terry Guo
@ 2011-09-01  6:46     ` Jakub Jelinek
  2011-09-01  7:10       ` Terry Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2011-09-01  6:46 UTC (permalink / raw)
  To: Terry Guo; +Cc: gcc-patches, ro, mikestump

On Thu, Sep 01, 2011 at 02:32:51PM +0800, Terry Guo wrote:
> FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile
> FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile
> UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o
> link 
> UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-c_compat_y_tst.o
> execute
> 
> On my machine, the error message is "error: width of 'a' exceeds its type".
> My GCC is cross built for arm-none-eabi based on upstream GCC 4.6. The
> command I run this case is simply as: "make check-gcc
> RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-m3
> compat.exp=struct-layout-1*". Are there anything else I should do to pass
> this case? 

Look into gcc/testsuite/gcc/gcc.log, search for struct-layout-1_generate.exe
and see whether -e has been passed to it?  If not, debug the tcl bits which
are supposed to pass it, but for some reason don't, if yes, look into the
generator under debugger why it generates the large enum bitfield bitsizes
anyway (you can cut'n'paste the *generate.exe command line, compile it with
-g and rerun it under debugger...).

	Jakub

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

* RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  6:46     ` Jakub Jelinek
@ 2011-09-01  7:10       ` Terry Guo
  2011-09-01  7:23         ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Terry Guo @ 2011-09-01  7:10 UTC (permalink / raw)
  To: 'Jakub Jelinek'; +Cc: gcc-patches, ro, mikestump

Hello Jakub,

> 
> On Thu, Sep 01, 2011 at 02:32:51PM +0800, Terry Guo wrote:
> > FAIL: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o compile
> > FAIL: gcc.dg/compat/struct-layout-1 c_compat_y_tst.o compile
> > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-
> c_compat_y_tst.o
> > link
> > UNRESOLVED: gcc.dg/compat/struct-layout-1 c_compat_x_tst.o-
> c_compat_y_tst.o
> > execute
> >
> > On my machine, the error message is "error: width of 'a' exceeds its
> type".
> > My GCC is cross built for arm-none-eabi based on upstream GCC 4.6.
> The
> > command I run this case is simply as: "make check-gcc
> > RUNTESTFLAGS="--target_board=arm-none-eabi-qemu/-mthumb/-mcpu=cortex-
> m3
> > compat.exp=struct-layout-1*". Are there anything else I should do to
> pass
> > this case?
> 
> Look into gcc/testsuite/gcc/gcc.log, search for struct-layout-
> 1_generate.exe
> and see whether -e has been passed to it?  If not, debug the tcl bits
> which
> are supposed to pass it, but for some reason don't, if yes, look into
> the
> generator under debugger why it generates the large enum bitfield
> bitsizes
> anyway (you can cut'n'paste the *generate.exe command line, compile it
> with
> -g and rerun it under debugger...).
> 

What you said is for compat/struct-layout-1.exp. What I said is for
compat/compat.exp. The case contains three files struct-layout-1_x.c,
struct-layout-1_y.c and struct-layout-1_main.c. All of them are independent
files and not generated by generate.exe. Meanwhile do you think it is
necessary to keep this cases in compat.exp given we already have
struct-layout-1.exp?

BR,
Terry





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

* Re: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  7:10       ` Terry Guo
@ 2011-09-01  7:23         ` Jakub Jelinek
  2011-09-01  7:46           ` Terry Guo
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2011-09-01  7:23 UTC (permalink / raw)
  To: Terry Guo; +Cc: gcc-patches, ro, mikestump

On Thu, Sep 01, 2011 at 03:09:34PM +0800, Terry Guo wrote:
> What you said is for compat/struct-layout-1.exp. What I said is for
> compat/compat.exp. The case contains three files struct-layout-1_x.c,
> struct-layout-1_y.c and struct-layout-1_main.c. All of them are independent
> files and not generated by generate.exe. Meanwhile do you think it is
> necessary to keep this cases in compat.exp given we already have
> struct-layout-1.exp?

The files are there to test the infrastructure.  I'd say just try
2011-09-01  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/compat/struct-layout-1_test.h: Decrease bitfield size
	to work even with -fshort-enums.

--- gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h	2008-09-05 12:54:32.000000000 +0200
+++ gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h	2011-09-01 09:21:20.200426411 +0200
@@ -1,5 +1 @@
-#if (__SIZEOF_INT__ >= 4) 
-T(0,enum E2 a:31;,B(0,a,e2_m1,e2_0))
-#else
-T(0,enum E2 a:15;,B(0,a,e2_m1,e2_0))
-#endif
+T(0,enum E2 a:7;,B(0,a,e2_m1,e2_0))

instead.

	Jakub

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

* RE: [Patch, testsuite] Skip case struct-layout-1 for targets using short enums.
  2011-09-01  7:23         ` Jakub Jelinek
@ 2011-09-01  7:46           ` Terry Guo
  0 siblings, 0 replies; 7+ messages in thread
From: Terry Guo @ 2011-09-01  7:46 UTC (permalink / raw)
  To: 'Jakub Jelinek'; +Cc: gcc-patches, ro, mikestump

Hello Jakub,

> The files are there to test the infrastructure.  I'd say just try
> 2011-09-01  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* gcc.dg/compat/struct-layout-1_test.h: Decrease bitfield size
> 	to work even with -fshort-enums.
> 
> --- gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h	2008-09-05
> 12:54:32.000000000 +0200
> +++ gcc/testsuite/gcc.dg/compat/struct-layout-1_test.h	2011-09-01
> 09:21:20.200426411 +0200
> @@ -1,5 +1 @@
> -#if (__SIZEOF_INT__ >= 4)
> -T(0,enum E2 a:31;,B(0,a,e2_m1,e2_0))
> -#else
> -T(0,enum E2 a:15;,B(0,a,e2_m1,e2_0))
> -#endif
> +T(0,enum E2 a:7;,B(0,a,e2_m1,e2_0))
> 
> instead.
> 

I just tried the patch, it works well. The case get passed. Would you please
commit it into trunk and GCC 4.6 branch? If you want me to do something
else, please let me know. Thanks.

BR,
Terry


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

end of thread, other threads:[~2011-09-01  7:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-01  1:40 [Patch, testsuite] Skip case struct-layout-1 for targets using short enums Terry Guo
2011-09-01  6:14 ` Jakub Jelinek
2011-09-01  6:34   ` Terry Guo
2011-09-01  6:46     ` Jakub Jelinek
2011-09-01  7:10       ` Terry Guo
2011-09-01  7:23         ` Jakub Jelinek
2011-09-01  7:46           ` Terry Guo

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).