[Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Dennis Pahl]

Hi,

in my project I need the enum size to be 4 bytes. Therefore in order to 
get no linker compatibility warnings, I also require newlib to use 4 
bytes for its enums.

4 byte enums can be achieved by specifying the following compiler flag 
"-fno-short-enums". This flag works fine for the source code of my 
project. Newlib, however, has a problem with this flag due to some hard 
coded compiler invocations which include the opposite flag "-fshort-enums".

A look in the mailing list archive revealed, that there already has been 
a discussion about this problem 
(https://sourceware.org/ml/newlib/2013/msg00183.html). The final advice 
from Michael Bruck, who started the discussion, was to remove these hard 
coded flags entirely from the build script files. Is there a reason why 
this did not happen?

With the removed hard coded flags the enum size can be configured by 
specifying
CFLAGS_FOR_TARGET= '-fno-short-enums' or '-fshort-enums'
CXXFLAGS_FOR_TARGET='-fno-short-enums' or '-fshort-enums'

By following Michael Bruck's advice I was able to build newlib with the 
wanted enum size.

Dennis Pahl

---

Here a patch with my adaptations, which remove the hard coded compiler 
flags:

commit d52c0a49ab9503718701977af451e0cb9164c084
Author: Dennis Pahl <dennis@pahl.de>
Date:   Sun Aug 28 13:48:28 2016 +0200

     removed the fix configuration of short enums for some build files. 
It is more appropriate if the external configuration decides whether the 
compiled files have short enums or not.

diff --git a/newlib/libc/stdio/Makefile.am b/newlib/libc/stdio/Makefile.am
index ea23de6..2a77b7b 100644
--- a/newlib/libc/stdio/Makefile.am
+++ b/newlib/libc/stdio/Makefile.am
@@ -254,44 +254,44 @@ include $(srcdir)/../../Makefile.shared
  if NEWLIB_NANO_FORMATTED_IO
  # Rules compiling small-footprint nano-formatted-io implementation.
  $(lpfx)nano-vfprintf.$(oext): nano-vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/nano-vfprintf.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/nano-vfprintf.c -o $@

  $(lpfx)nano-vfprintf_i.$(oext): nano-vfprintf_i.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/nano-vfprintf_i.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/nano-vfprintf_i.c -o $@

  $(lpfx)nano-vfprintf_float.$(oext): nano-vfprintf_float.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/nano-vfprintf_float.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/nano-vfprintf_float.c -o $@

  $(lpfx)nano-svfprintf.$(oext): nano-vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DSTRING_ONLY -c 
$(srcdir)/nano-vfprintf.c -o $@
+	$(LIB_COMPILE) -DSTRING_ONLY -c $(srcdir)/nano-vfprintf.c -o $@
  endif

  # This rule is needed so that libtool compiles vfiprintf before vfprintf.
  # Otherwise libtool moves vfprintf.o and subsequently can't find it.

  $(lpfx)vfprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)vfiprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)svfprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)svfiprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -DSTRING_ONLY -c 
$(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)vfwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)vfiwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)svfwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)svfiwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -DSTRING_ONLY -c 
$(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@

  if NEWLIB_NANO_FORMATTED_IO
  # Rules compiling small-footprint nano-formatted-io implementation.
diff --git a/newlib/libc/stdio/Makefile.in b/newlib/libc/stdio/Makefile.in
index 8ea5f0a..8a7acd3 100644
--- a/newlib/libc/stdio/Makefile.in
+++ b/newlib/libc/stdio/Makefile.in
@@ -1882,43 +1882,43 @@ docbook: $(DOCBOOK_OUT_FILES)

  # Rules compiling small-footprint nano-formatted-io implementation.
  @NEWLIB_NANO_FORMATTED_IO_TRUE@$(lpfx)nano-vfprintf.$(oext): 
nano-vfprintf.c
-@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -fshort-enums -c 
$(srcdir)/nano-vfprintf.c -o $@
+@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -c 
$(srcdir)/nano-vfprintf.c -o $@

  @NEWLIB_NANO_FORMATTED_IO_TRUE@$(lpfx)nano-vfprintf_i.$(oext): 
nano-vfprintf_i.c
-@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -fshort-enums -c 
$(srcdir)/nano-vfprintf_i.c -o $@
+@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -c 
$(srcdir)/nano-vfprintf_i.c -o $@

  @NEWLIB_NANO_FORMATTED_IO_TRUE@$(lpfx)nano-vfprintf_float.$(oext): 
nano-vfprintf_float.c
-@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -fshort-enums -c 
$(srcdir)/nano-vfprintf_float.c -o $@
+@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -c 
$(srcdir)/nano-vfprintf_float.c -o $@

  @NEWLIB_NANO_FORMATTED_IO_TRUE@$(lpfx)nano-svfprintf.$(oext): 
nano-vfprintf.c
-@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -fshort-enums 
-DSTRING_ONLY -c $(srcdir)/nano-vfprintf.c -o $@
+@NEWLIB_NANO_FORMATTED_IO_TRUE@	$(LIB_COMPILE) -DSTRING_ONLY -c 
$(srcdir)/nano-vfprintf.c -o $@

  # This rule is needed so that libtool compiles vfiprintf before vfprintf.
  # Otherwise libtool moves vfprintf.o and subsequently can't find it.

  $(lpfx)vfprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)vfiprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)svfprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)svfiprintf.$(oext): vfprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -DSTRING_ONLY -c 
$(srcdir)/vfprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -DSTRING_ONLY -c $(srcdir)/vfprintf.c -o $@

  $(lpfx)vfwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)vfiwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)svfwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@

  $(lpfx)svfiwprintf.$(oext): vfwprintf.c
-	$(LIB_COMPILE) -fshort-enums -DINTEGER_ONLY -DSTRING_ONLY -c 
$(srcdir)/vfwprintf.c -o $@
+	$(LIB_COMPILE) -DINTEGER_ONLY -DSTRING_ONLY -c $(srcdir)/vfwprintf.c -o $@

  # Rules compiling small-footprint nano-formatted-io implementation.
  @NEWLIB_NANO_FORMATTED_IO_TRUE@$(lpfx)nano-vfscanf.$(oext): nano-vfscanf.c
diff --git a/newlib/libc/stdlib/Makefile.am b/newlib/libc/stdlib/Makefile.am
index 2d45d10..5e134e3 100644
--- a/newlib/libc/stdlib/Makefile.am
+++ b/newlib/libc/stdlib/Makefile.am
@@ -307,7 +307,7 @@ $(lpfx)dtoa.$(oext): dtoa.c mprec.h
  $(lpfx)ldtoa.$(oext): ldtoa.c mprec.h
  $(lpfx)ecvtbuf.$(oext): ecvtbuf.c mprec.h
  $(lpfx)mbtowc_r.$(oext): mbtowc_r.c mbctype.h
-	$(LIB_COMPILE) -c -fshort-enums $(srcdir)/mbtowc_r.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/mbtowc_r.c -o $@

  $(lpfx)mprec.$(oext): mprec.c mprec.h
  $(lpfx)strtod.$(oext): strtod.c mprec.h
diff --git a/newlib/libc/stdlib/Makefile.in b/newlib/libc/stdlib/Makefile.in
index 466ab6d..88a0db5 100644
--- a/newlib/libc/stdlib/Makefile.in
+++ b/newlib/libc/stdlib/Makefile.in
@@ -1560,7 +1560,7 @@ $(lpfx)dtoa.$(oext): dtoa.c mprec.h
  $(lpfx)ldtoa.$(oext): ldtoa.c mprec.h
  $(lpfx)ecvtbuf.$(oext): ecvtbuf.c mprec.h
  $(lpfx)mbtowc_r.$(oext): mbtowc_r.c mbctype.h
-	$(LIB_COMPILE) -c -fshort-enums $(srcdir)/mbtowc_r.c -o $@
+	$(LIB_COMPILE) -c $(srcdir)/mbtowc_r.c -o $@

  $(lpfx)mprec.$(oext): mprec.c mprec.h
  $(lpfx)strtod.$(oext): strtod.c mprec.h

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Corinna Vinschen]

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

On Aug 28 16:31, Dennis Pahl wrote:
> Hi,
> 
> in my project I need the enum size to be 4 bytes. Therefore in order to get
> no linker compatibility warnings, I also require newlib to use 4 bytes for
> its enums.
> 
> 4 byte enums can be achieved by specifying the following compiler flag
> "-fno-short-enums". This flag works fine for the source code of my project.
> Newlib, however, has a problem with this flag due to some hard coded
> compiler invocations which include the opposite flag "-fshort-enums".
> 
> A look in the mailing list archive revealed, that there already has been a
> discussion about this problem
> (https://sourceware.org/ml/newlib/2013/msg00183.html). The final advice from
> Michael Bruck, who started the discussion, was to remove these hard coded
> flags entirely from the build script files. Is there a reason why this did
> not happen?
> 
> With the removed hard coded flags the enum size can be configured by
> specifying
> CFLAGS_FOR_TARGET= '-fno-short-enums' or '-fshort-enums'
> CXXFLAGS_FOR_TARGET='-fno-short-enums' or '-fshort-enums'
> 
> By following Michael Bruck's advice I was able to build newlib with the
> wanted enum size.

The patch is ok with me, what do other user's of ARM think?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Joel Sherrill]

On August 30, 2016 4:47:18 AM CDT, Corinna Vinschen <vinschen@redhat.com> wrote:
>On Aug 28 16:31, Dennis Pahl wrote:
>> Hi,
>> 
>> in my project I need the enum size to be 4 bytes. Therefore in order
>to get
>> no linker compatibility warnings, I also require newlib to use 4
>bytes for
>> its enums.
>> 
>> 4 byte enums can be achieved by specifying the following compiler
>flag
>> "-fno-short-enums". This flag works fine for the source code of my
>project.
>> Newlib, however, has a problem with this flag due to some hard coded
>> compiler invocations which include the opposite flag "-fshort-enums".
>> 
>> A look in the mailing list archive revealed, that there already has
>been a
>> discussion about this problem
>> (https://sourceware.org/ml/newlib/2013/msg00183.html). The final
>advice from
>> Michael Bruck, who started the discussion, was to remove these hard
>coded
>> flags entirely from the build script files. Is there a reason why
>this did
>> not happen?
>> 
>> With the removed hard coded flags the enum size can be configured by
>> specifying
>> CFLAGS_FOR_TARGET= '-fno-short-enums' or '-fshort-enums'
>> CXXFLAGS_FOR_TARGET='-fno-short-enums' or '-fshort-enums'
>> 
>> By following Michael Bruck's advice I was able to build newlib with
>the
>> wanted enum size.
>
>The patch is ok with me, what do other user's of ARM think?

I assume the default is 32 bit.

Does configuring newlib for small size automatically trigger the small enum size?

Someone who works more intimately with the arm and especially the smaller ones needs to comment

--joel

>
>Corinna

--joel

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Richard Earnshaw (lists)]

[Sorry, one more try]
[Apologies if you get this twice, I think I had a mail configuration
problem earlier today]

On 30/08/16 10:47, Corinna Vinschen wrote:
> On Aug 28 16:31, Dennis Pahl wrote:
>> Hi,
>>
>> in my project I need the enum size to be 4 bytes. Therefore in order to get
>> no linker compatibility warnings, I also require newlib to use 4 bytes for
>> its enums.
>>
>> 4 byte enums can be achieved by specifying the following compiler flag
>> "-fno-short-enums". This flag works fine for the source code of my project.
>> Newlib, however, has a problem with this flag due to some hard coded
>> compiler invocations which include the opposite flag "-fshort-enums".
>>
>> A look in the mailing list archive revealed, that there already has been a
>> discussion about this problem
>> (https://sourceware.org/ml/newlib/2013/msg00183.html). The final advice from
>> Michael Bruck, who started the discussion, was to remove these hard coded
>> flags entirely from the build script files. Is there a reason why this did
>> not happen?

Corinna asked for a patch, but none was forthcoming AFAICT.  This is
OpenSource: you can't expect others to do the work for you.

>>
>> With the removed hard coded flags the enum size can be configured by
>> specifying
>> CFLAGS_FOR_TARGET= '-fno-short-enums' or '-fshort-enums'
>> CXXFLAGS_FOR_TARGET='-fno-short-enums' or '-fshort-enums'
>>
>> By following Michael Bruck's advice I was able to build newlib with the
>> wanted enum size.
> 
> The patch is ok with me, what do other user's of ARM think?
> 
> 
> Corinna
> 

I've just done some archaeology with some colleagues.  It appears that
the initial addition of -fshort-enums was done by Jeff way back in 2002
(well before newlib nano).  It also appears that it has been done only
for selected files - I presume this is because it is 'known' that these
functions do not export any dependencies on the size of an enum, so it
shaves a few bytes off their internal needs.

We've never noticed this before because the bare-metal ARM builds use
-fshort-enums by default, otherwise we would see build conflicts today
since gcc for ARM emits build attributes that describe the selection and
the linker will then fault incompatible object files[1].

So based on the above, I agree that this looks to be a sensible patch.
We'll have to check carefully that it doesn't break anything, however,
or cause any major code size regressions in existing configurations.

R.



[1]  Technically build attributes support describing more complex cases,
such as use enums for internal purposes only, but GCC has no way of
describing generating the complex cases or validating such claims by the
user.

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Corinna Vinschen]

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

On Aug 30 14:07, Richard Earnshaw (lists) wrote:
> On 30/08/16 10:47, Corinna Vinschen wrote:
> > On Aug 28 16:31, Dennis Pahl wrote:
> >> A look in the mailing list archive revealed, that there already has been a
> >> discussion about this problem
> >> (https://sourceware.org/ml/newlib/2013/msg00183.html). The final advice from
> >> Michael Bruck, who started the discussion, was to remove these hard coded
> >> flags entirely from the build script files. Is there a reason why this did
> >> not happen?
> 
> Corinna asked for a patch, but none was forthcoming AFAICT.  This is
> OpenSource: you can't expect others to do the work for you.
> 
> >> With the removed hard coded flags the enum size can be configured by
> >> specifying
> >> CFLAGS_FOR_TARGET= '-fno-short-enums' or '-fshort-enums'
> >> CXXFLAGS_FOR_TARGET='-fno-short-enums' or '-fshort-enums'
> >>
> >> By following Michael Bruck's advice I was able to build newlib with the
> >> wanted enum size.
> > 
> > The patch is ok with me, what do other user's of ARM think?
> 
> I've just done some archaeology with some colleagues.  It appears that
> the initial addition of -fshort-enums was done by Jeff way back in 2002
> (well before newlib nano).  It also appears that it has been done only
> for selected files - I presume this is because it is 'known' that these
> functions do not export any dependencies on the size of an enum, so it
> shaves a few bytes off their internal needs.
> 
> We've never noticed this before because the bare-metal ARM builds use
> -fshort-enums by default, otherwise we would see build conflicts today
> since gcc for ARM emits build attributes that describe the selection and
> the linker will then fault incompatible object files[1].
> 
> So based on the above, I agree that this looks to be a sensible patch.
> We'll have to check carefully that it doesn't break anything, however,
> or cause any major code size regressions in existing configurations.

Yeah, "somebody" would have to do that.  However, if the enums in the
affected files are only used internally, what could possible go wrong?
*duck*

On a more serious note, if, as you say, memory-conscious bare-metal ARM
builds use -fshort-enums anyway, they don't need the flag hard coded for
a handful of files in the lib since the entire lib will use this flag.

While builds which don't use -fshort-enums are in all likelyhood not as
memory-conscious anyway and won't profit much from the few lib files
built with this flag.

So, from a neutral perspective, I'd say the special handling of these
files only makes marginal sense and could go away, isn't it?  Am I
missing something?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Schwarz, Konrad]

> -----Original Message-----
> From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> On Behalf Of Corinna Vinschen
> The patch is ok with me, what do other user's of ARM think?

This is a surprisingly tough question to answer.

The AAPCS for ABI release 2.10, (current ARM ABI) says:

	This ABI delegates a choice of representation of enumerated types to a platform ABI (whether defined by a
	standard or by custom and practice) or to an interface contract if there is no defined platform ABI.

It also notes:
	Under this ABI, these statements allow a header file that describes the interface to a portable binary package to
	force its clients, in a portable, strictly-conforming manner, to adopt a 32-bit signed (int/long) representation of
	values of enumerated type (by defining a negative enumerator, a positive one, and ensuring the range of
	enumerators spans more than 16 bits but not more than 32).

The ABI-Addenda defines a specific Tag_ABI_enum_size value for this usage, in addition to the cases [no enums permitted],
[smallest container], and [32-bit enums].

It used to say (in r2.05):
	The type of the storage container for an enumerated type is the smallest integer type that contains all the enumerated values.

GCC's manual has the following to say (this is target-independent):

     *Warning:* the '-fshort-enums' switch causes GCC to generate code
     that is not binary compatible with code generated without that
     switch.  Use it to conform to a non-default application binary
     interface.

So GCC seems to think that -fshort-enums should be used only in exceptional circumstances.

I'm unclear where newlib uses enums in its ABI, but the AAPCS suggestion of enforcing minimal enum sizes via appropriately
sized integer constants (and ensuring that all enums in newlib have exactly 32 bits) and using the right
Tag_ABI_enum_size should allow code compiled with -fshort-enums or with -fno-short-enums to link to newlib.

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Corinna Vinschen]

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

On Aug 30 13:16, Schwarz, Konrad wrote:
> > -----Original Message-----
> > From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> > On Behalf Of Corinna Vinschen
> > The patch is ok with me, what do other user's of ARM think?
> 
> This is a surprisingly tough question to answer.
> 
> The AAPCS for ABI release 2.10, (current ARM ABI) says:
> 
> 	This ABI delegates a choice of representation of enumerated
> 	types to a platform ABI (whether defined by a standard or by
> 	custom and practice) or to an interface contract if there is no
> 	defined platform ABI.
> 
> It also notes: Under this ABI, these statements allow a header file
> that describes the interface to a portable binary package to force its
> clients, in a portable, strictly-conforming manner, to adopt a 32-bit
> signed (int/long) representation of values of enumerated type (by
> defining a negative enumerator, a positive one, and ensuring the range
> of enumerators spans more than 16 bits but not more than 32).
> 
> The ABI-Addenda defines a specific Tag_ABI_enum_size value for this
> usage, in addition to the cases [no enums permitted], [smallest
> container], and [32-bit enums].
> 
> It used to say (in r2.05): The type of the storage container for an
> enumerated type is the smallest integer type that contains all the
> enumerated values.
> 
> GCC's manual has the following to say (this is target-independent):
> 
>      *Warning:* the '-fshort-enums' switch causes GCC to generate code
>      that is not binary compatible with code generated without that
>      switch.  Use it to conform to a non-default application binary
>      interface.
> 
> So GCC seems to think that -fshort-enums should be used only in
> exceptional circumstances.
> 
> I'm unclear where newlib uses enums in its ABI, but the AAPCS
> suggestion of enforcing minimal enum sizes via appropriately sized
> integer constants (and ensuring that all enums in newlib have exactly
> 32 bits) and using the right Tag_ABI_enum_size should allow code
> compiled with -fshort-enums or with -fno-short-enums to link to
> newlib.

Given that, per Richard, the enum usage in the affected files is
internal a change here is apparently not ABI-relevant.  From my
rather neutral stance it looks like a pretty safe change so far...


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Pavel Pisa]

Hello Corinna and all others,

On Tuesday 30 of August 2016 20:27:57 Corinna Vinschen wrote:
> Given that, per Richard, the enum usage in the affected files is
> internal a change here is apparently not ABI-relevant.  From my
> rather neutral stance it looks like a pretty safe change so far...

When I look to

  newlib/libc/stdlib/mbtowc_r.c

then it seems that tables using enums as base type can have considerable
impact on size for tiny/nano targets.

typedef enum { ESCAPE, DOLLAR, BRACKET, AT, B, J,
               NUL, JIS_CHAR, OTHER, JIS_C_NUM } JIS_CHAR_TYPE;
typedef enum { ASCII, JIS, A_ESC, A_ESC_DL, JIS_1, J_ESC, J_ESC_BR,
               INV, JIS_S_NUM } JIS_STATE;
typedef enum { COPY_A, COPY_J1, COPY_J2, MAKE_A, NOOP, EMPTY, ERROR } JIS_ACTION;

But I am not sure if using -fshort-enums is the right way to solve that.
If we can agree that enums in C are mainly usesfull for unique values
in the set assignment but in the fact there is no check that enum
values are assigned to the variables of same type, then much better
solution is to count cases by hand and decide about type.

In this particular case, I expect that next is most space conserving

enum { ESCAPE, DOLLAR, BRACKET, AT, B, J, NUL, JIS_CHAR, OTHER, JIS_C_NUM };
typedef int_least8_t JIS_CHAR_TYPE;

enum { ASCII, JIS, A_ESC, A_ESC_DL, JIS_1, J_ESC, J_ESC_BR,
     INV, JIS_S_NUM };
typedef int_least8_t JIS_STATE;

enum { COPY_A, COPY_J1, COPY_J2, MAKE_A, NOOP, EMPTY, ERROR };
typedef int_least8_t JIS_ACTION;

What makes me worry for waste of memory in this particular file is
that tables go to data section

#ifndef  __CYGWIN__
static JIS_STATE JIS_state_table[JIS_S_NUM][JIS_C_NUM] = {

I think, that they should be
static const JIS_STATE JIS_state_table[JIS_S_NUM][JIS_C_NUM] = {

Same for

static mbtowc_p __cp_xxx_mbtowc[26] = {

For newlib/libc/stdio/vfprintf.c, it looks like enums are used only as
variables inside functions. The size is not big deal for local variables.
If registers halves, quarters etc. can used for multiple variables
on some targets then space for enum types can be defined as uint_fast8_t .
But generally difference is minimal.

Best wishes,

                Pavel

RE: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Schwarz, Konrad]

> -----Original Message-----
> From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
> On Behalf Of Corinna Vinschen

> > I'm unclear where newlib uses enums in its ABI, but the AAPCS
> > suggestion of enforcing minimal enum sizes via appropriately sized
> > integer constants (and ensuring that all enums in newlib have exactly
> > 32 bits) and using the right Tag_ABI_enum_size should allow code
> > compiled with -fshort-enums or with -fno-short-enums to link to
> > newlib.
> 
> Given that, per Richard, the enum usage in the affected files is
> internal a change here is apparently not ABI-relevant.  From my rather
> neutral stance it looks like a pretty safe change so far...

My admittedly theoretical understanding is that if the change affects
only the implementation, not the ABI, it would equally be possible
to mark the object files in newlib as, for example, not using enums
in its ABI.  This would allow for space-efficient representation
of internal tables and would seem the best solution and the one
intended by the AAPCS.

However, Richard seems to indicate that the GCC tool chain does
not support this.  I'm not sure what is missing here -- e.g.,
it is often possible to slip in an __asm__ to create an entry
in some section used solely by the linker, but maybe this
trick doesn't work in this case, or ld doesn't understand the
special semantics.

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Richard Earnshaw (lists)]

On 31/08/16 08:55, Schwarz, Konrad wrote:
>> -----Original Message-----
>> From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org]
>> On Behalf Of Corinna Vinschen
> 
>>> I'm unclear where newlib uses enums in its ABI, but the AAPCS
>>> suggestion of enforcing minimal enum sizes via appropriately sized
>>> integer constants (and ensuring that all enums in newlib have exactly
>>> 32 bits) and using the right Tag_ABI_enum_size should allow code
>>> compiled with -fshort-enums or with -fno-short-enums to link to
>>> newlib.
>>
>> Given that, per Richard, the enum usage in the affected files is
>> internal a change here is apparently not ABI-relevant.  From my rather
>> neutral stance it looks like a pretty safe change so far...
> 
> My admittedly theoretical understanding is that if the change affects
> only the implementation, not the ABI, it would equally be possible
> to mark the object files in newlib as, for example, not using enums
> in its ABI.  This would allow for space-efficient representation
> of internal tables and would seem the best solution and the one
> intended by the AAPCS.
> 
> However, Richard seems to indicate that the GCC tool chain does
> not support this.  I'm not sure what is missing here -- e.g.,
> it is often possible to slip in an __asm__ to create an entry
> in some section used solely by the linker, but maybe this
> trick doesn't work in this case, or ld doesn't understand the
> special semantics.
> 


GCC/GAS/LD on ARM systems emit Build Attributes[1].  One of these
attributes describes the way in which the file depends on how enum types
are handled (since in an ABI it *can* affect whether the files are
compatible with each other).  Unfortunately, GCC has a limitation in
that it only supports two of the four possible attribute values for this
particular attribute since we can't get at enough details in the
mid/front end to validate the user's code complies with the attribute
settings.

Note that with recent GCC you could use the following trick to limit the
size of the internal enums without changing the public interface model.

#pragma GCC push_options
#pragma GCC optimize ("-fshort-enums")
enum {
  a, b, c
} w;
#pragma GCC pop_options

int x = sizeof (w);


enum {
  d, e, f
} y;

int z = sizeof (y);

R.


[1]
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0045e/IHI0045E_ABI_addenda.pdf

RE: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Schwarz, Konrad]

> -----Original Message-----
> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
> Sent: Wednesday, August 31, 2016 1:15 PM
> To: Schwarz, Konrad (CT RDA ITP SES-DE); newlib@sourceware.org
> Subject: Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums)
> | Remove hard coded short enum flag from the build scripts?

> GCC/GAS/LD on ARM systems emit Build Attributes[1].  One of these
> attributes describes the way in which the file depends on how enum
> types are handled (since in an ABI it *can* affect whether the files
> are compatible with each other).  Unfortunately, GCC has a limitation
> in that it only supports two of the four possible attribute values for
> this particular attribute since we can't get at enough details in the
> mid/front end to validate the user's code complies with the attribute
> settings.

But wouldn't it suffice to allow the user to select (or override) the
build attribute to set on the file, e.g., via a compiler command line switch?

Obviously, this requires the compiler to trust the user.

Conversely, as there is no way in (standard) C to distinguish
an ABI from interfaces internal to the implementation,
any scheme to determine this build attribute automatically
cannot be fully satisfactory, no?

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Richard Earnshaw (lists)]

On 31/08/16 12:36, Schwarz, Konrad wrote:
>> -----Original Message-----
>> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
>> Sent: Wednesday, August 31, 2016 1:15 PM
>> To: Schwarz, Konrad (CT RDA ITP SES-DE); newlib@sourceware.org
>> Subject: Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums)
>> | Remove hard coded short enum flag from the build scripts?
> 
>> GCC/GAS/LD on ARM systems emit Build Attributes[1].  One of these
>> attributes describes the way in which the file depends on how enum
>> types are handled (since in an ABI it *can* affect whether the files
>> are compatible with each other).  Unfortunately, GCC has a limitation
>> in that it only supports two of the four possible attribute values for
>> this particular attribute since we can't get at enough details in the
>> mid/front end to validate the user's code complies with the attribute
>> settings.
> 
> But wouldn't it suffice to allow the user to select (or override) the
> build attribute to set on the file, e.g., via a compiler command line switch?
> 

Yes, but 1) we have no builds of GCC available *today* that can do that
and 2) experience has shown that such 'power user' options generally
confuse most users and lead to more problems than they really solve
unless they can validate the user's assertions.

> Obviously, this requires the compiler to trust the user.
> 
> Conversely, as there is no way in (standard) C to distinguish
> an ABI from interfaces internal to the implementation,
> any scheme to determine this build attribute automatically
> cannot be fully satisfactory, no?
> 

No, this is nothing to do with standard C.  The C standard just thinks
about enums at the conceptual level.  It doesn't grubby its hands with
nasty details like ABIs or layout in general...  The idea that this
might be changed in different translation units is completely off-piste.

R.

RE: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Schwarz, Konrad]

> -----Original Message-----
> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
> Sent: Wednesday, August 31, 2016 2:38 PM
> To: Schwarz, Konrad (CT RDA ITP SES-DE); newlib@sourceware.org
> Subject: Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums)
> | Remove hard coded short enum flag from the build scripts?


> > But wouldn't it suffice to allow the user to select (or override) the
> > build attribute to set on the file, e.g., via a compiler command line
> switch?
> >
> 
> Yes, but 1) we have no builds of GCC available *today* that can do that
> and 2) experience has shown that such 'power user' options generally
> confuse most users and lead to more problems than they really solve
> unless they can validate the user's assertions.
> 
> > Obviously, this requires the compiler to trust the user.
> >
> > Conversely, as there is no way in (standard) C to distinguish an ABI
> > from interfaces internal to the implementation, any scheme to
> > determine this build attribute automatically cannot be fully
> > satisfactory, no?
> >
> 
> No, this is nothing to do with standard C.  The C standard just thinks
> about enums at the conceptual level.  It doesn't grubby its hands with
> nasty details like ABIs or layout in general...  The idea that this
> might be changed in different translation units is completely off-
> piste.

The corollary being that the only way to do this properly is via a compiler
switch, even it is confusing to non-power users.  (Since when has
GCC limited itself to the needs of non-power users?)

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Richard Earnshaw (lists)]

On 31/08/16 13:44, Schwarz, Konrad wrote:
>> -----Original Message-----
>> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
>> Sent: Wednesday, August 31, 2016 2:38 PM
>> To: Schwarz, Konrad (CT RDA ITP SES-DE); newlib@sourceware.org
>> Subject: Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums)
>> | Remove hard coded short enum flag from the build scripts?
> 
> 
>>> But wouldn't it suffice to allow the user to select (or override) the
>>> build attribute to set on the file, e.g., via a compiler command line
>> switch?
>>>
>>
>> Yes, but 1) we have no builds of GCC available *today* that can do that
>> and 2) experience has shown that such 'power user' options generally
>> confuse most users and lead to more problems than they really solve
>> unless they can validate the user's assertions.
>>
>>> Obviously, this requires the compiler to trust the user.
>>>
>>> Conversely, as there is no way in (standard) C to distinguish an ABI
>>> from interfaces internal to the implementation, any scheme to
>>> determine this build attribute automatically cannot be fully
>>> satisfactory, no?
>>>
>>
>> No, this is nothing to do with standard C.  The C standard just thinks
>> about enums at the conceptual level.  It doesn't grubby its hands with
>> nasty details like ABIs or layout in general...  The idea that this
>> might be changed in different translation units is completely off-
>> piste.
> 
> The corollary being that the only way to do this properly is via a compiler
> switch, even it is confusing to non-power users.  (Since when has
> GCC limited itself to the needs of non-power users?)
> 

I think the best way to do this is using the pragmas I previously
mentioned.  This gives far more control (power) than the command-line
switch and has a degree of backward compatibility with some existing
versions of GCC.

#define PUSH_PACK_ENUMS _Pragma("GCC push_options") _Pragma ("GCC
optimize (\"-fshort-enums\")")
#define POP_PACK_ENUMS _Pragma("GCC pop_options")


Now some pre-processor testing for support of the GCC pragmas will allow
us to write

PUSH_PACK_ENUMS
enum {a, b, c} w;
POP_PACK_ENUMS

int x = sizeof (w);

enum {
  d, e, f
} y;

int z = sizeof (y);

Or something like that.

Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums) | Remove hard coded short enum flag from the build scripts?

[Richard Earnshaw (lists)]

On 31/08/16 15:33, Richard Earnshaw (lists) wrote:
> On 31/08/16 13:44, Schwarz, Konrad wrote:
>>> -----Original Message-----
>>> From: Richard Earnshaw (lists) [mailto:Richard.Earnshaw@arm.com]
>>> Sent: Wednesday, August 31, 2016 2:38 PM
>>> To: Schwarz, Konrad (CT RDA ITP SES-DE); newlib@sourceware.org
>>> Subject: Re: ARM-only [Patch] Allow 4 byte enum size (-fno-short-enums)
>>> | Remove hard coded short enum flag from the build scripts?
>>
>>
>>>> But wouldn't it suffice to allow the user to select (or override) the
>>>> build attribute to set on the file, e.g., via a compiler command line
>>> switch?
>>>>
>>>
>>> Yes, but 1) we have no builds of GCC available *today* that can do that
>>> and 2) experience has shown that such 'power user' options generally
>>> confuse most users and lead to more problems than they really solve
>>> unless they can validate the user's assertions.
>>>
>>>> Obviously, this requires the compiler to trust the user.
>>>>
>>>> Conversely, as there is no way in (standard) C to distinguish an ABI
>>>> from interfaces internal to the implementation, any scheme to
>>>> determine this build attribute automatically cannot be fully
>>>> satisfactory, no?
>>>>
>>>
>>> No, this is nothing to do with standard C.  The C standard just thinks
>>> about enums at the conceptual level.  It doesn't grubby its hands with
>>> nasty details like ABIs or layout in general...  The idea that this
>>> might be changed in different translation units is completely off-
>>> piste.
>>
>> The corollary being that the only way to do this properly is via a compiler
>> switch, even it is confusing to non-power users.  (Since when has
>> GCC limited itself to the needs of non-power users?)
>>
> 
> I think the best way to do this is using the pragmas I previously
> mentioned.  This gives far more control (power) than the command-line
> switch and has a degree of backward compatibility with some existing
> versions of GCC.
> 
> #define PUSH_PACK_ENUMS _Pragma("GCC push_options") _Pragma ("GCC
> optimize (\"-fshort-enums\")")
> #define POP_PACK_ENUMS _Pragma("GCC pop_options")
> 
> 
> Now some pre-processor testing for support of the GCC pragmas will allow
> us to write
> 
> PUSH_PACK_ENUMS
> enum {a, b, c} w;
> POP_PACK_ENUMS
> 
> int x = sizeof (w);
> 
> enum {
>   d, e, f
> } y;
> 
> int z = sizeof (y);
> 
> Or something like that.
> 


And even cleaner (and working with even more versions of gcc):

#define packed_enum enum __attribute__ ((__packed__))

packed_enum {a, b, c, m=260} w;

int x = sizeof (w);
int t = __alignof(w);

enum {
  d, e, f
} y;

int z = sizeof (y);