| Message ID | 1491219150-25075-1-git-send-email-sebastian.huber@embedded-brains.de |
|---|---|
| State | New |
| Headers | show |
On 3 April 2017 13:32:30 CEST, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote:
>Allow targets to define the default for the short enums option.
Does this work nowadays?
About 10 years ago I did this for some arm and had to force TREE_CODE at al to 16 bit manually, IIRC.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34205
On 04/04/17 09:43, Bernhard Reutner-Fischer wrote: > On 3 April 2017 13:32:30 CEST, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: >> Allow targets to define the default for the short enums option. > Does this work nowadays? > About 10 years ago I did this for some arm and had to force TREE_CODE at al to 16 bit manually, IIRC. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34205 The short enums work fine on ARM. I want to get rid of them not due to compiler bugs.
On Tue, Apr 4, 2017 at 8:43 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote: > On 3 April 2017 13:32:30 CEST, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: >>Allow targets to define the default for the short enums option. > > Does this work nowadays? > About 10 years ago I did this for some arm and had to force TREE_CODE at al to 16 bit manually, IIRC. > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=34205 I don't see why it won't work in cross-compilers and on Linux. the bug report that you allude to seems to suggest that bootstrap was broken with -fshort-enums i.e. -fshort-enums on a host. I don't see how that's going to be possible without major work in the software but for cross-compilers targeting embedded platforms I see no reason why -fshort-enums won't work. regards Ramana
On Mon, Apr 3, 2017 at 12:32 PM, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: > Allow targets to define the default for the short enums option. > > gcc/ > > * config/arm/arm.c: (ARM_DEFAULT_SHORT_ENUMS): Provide default > definition. > * config/arm/rtems.h (ARM_DEFAULT_SHORT_ENUMS) Define. > --- > gcc/config/arm/arm.c | 6 +++++- > gcc/config/arm/rtems.h | 2 ++ > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index b24143e..33d3834 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -26547,11 +26547,15 @@ arm_promote_function_mode (const_tree type ATTRIBUTE_UNUSED, > } > > /* AAPCS based ABIs use short enums by default. */ > +#ifndef ARM_DEFAULT_SHORT_ENUMS > +#define ARM_DEFAULT_SHORT_ENUMS \ > + (TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX) > +#endif This belongs in arm.h rather than in arm.c > > static bool > arm_default_short_enums (void) > { > - return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; > + return ARM_DEFAULT_SHORT_ENUMS; > } > > > diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h > index 53cd987..b34bbe8 100644 > --- a/gcc/config/arm/rtems.h > +++ b/gcc/config/arm/rtems.h > @@ -27,3 +27,5 @@ > builtin_assert ("system=rtems"); \ > TARGET_BPABI_CPP_BUILTINS(); \ > } while (0) > + > +#define ARM_DEFAULT_SHORT_ENUMS false It's a change in ABI for the RTEMS platform but it certainly needs a documentation update in the release notes . Also, is it necessary that you need this in for GCC-7 or can you wait for stage-1 since we are in regression fixes mode ? Ramana > -- > 1.8.4.5 >
On 04/04/17 11:00, Ramana Radhakrishnan wrote: >> > static bool >> > arm_default_short_enums (void) >> > { >> >- return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; >> >+ return ARM_DEFAULT_SHORT_ENUMS; >> > } >> > >> > >> >diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h >> >index 53cd987..b34bbe8 100644 >> >--- a/gcc/config/arm/rtems.h >> >+++ b/gcc/config/arm/rtems.h >> >@@ -27,3 +27,5 @@ >> > builtin_assert ("system=rtems"); \ >> > TARGET_BPABI_CPP_BUILTINS(); \ >> > } while (0) >> >+ >> >+#define ARM_DEFAULT_SHORT_ENUMS false > It's a change in ABI for the RTEMS platform but it certainly needs a > documentation update in the release notes . Also, is it necessary that > you need this in for GCC-7 or can you wait for stage-1 since we are in > regression fixes mode ? For RTEMS, ABI changes are not really critical. I would like to get this into GCC 6.4. For GCC 7 its not urgent.
[dropping devel at rtems dot org as I don't want more bounces] On Tue, Apr 4, 2017 at 10:07 AM, Sebastian Huber <sebastian.huber@embedded-brains.de> wrote: > On 04/04/17 11:00, Ramana Radhakrishnan wrote: >>> >>> > static bool >>> > arm_default_short_enums (void) >>> > { >>> >- return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; >>> >+ return ARM_DEFAULT_SHORT_ENUMS; >>> > } >>> > >>> > >>> >diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h >>> >index 53cd987..b34bbe8 100644 >>> >--- a/gcc/config/arm/rtems.h >>> >+++ b/gcc/config/arm/rtems.h >>> >@@ -27,3 +27,5 @@ >>> > builtin_assert ("system=rtems"); \ >>> > TARGET_BPABI_CPP_BUILTINS(); \ >>> > } while (0) >>> >+ >>> >+#define ARM_DEFAULT_SHORT_ENUMS false >> >> It's a change in ABI for the RTEMS platform but it certainly needs a >> documentation update in the release notes . Also, is it necessary that >> you need this in for GCC-7 or can you wait for stage-1 since we are in >> regression fixes mode ? > > > For RTEMS, ABI changes are not really critical. I would like to get this > into GCC 6.4. For GCC 7 its not urgent. > The usual policy is not to have ABI changes within sub-releases of a GCC release cycle. However if the RTEMs community is happy with it, I have no particular objections. I would however strongly suggest that if you are fixing it for GCC 6.4 to then fix it for GCC-7 *and* document it in the release notes. regards Ramana
On 04/04/17 12:41, Ramana Radhakrishnan wrote: > [dropping devel at rtems dot org as I don't want more bounces] > > On Tue, Apr 4, 2017 at 10:07 AM, Sebastian Huber > <sebastian.huber@embedded-brains.de> wrote: >> >On 04/04/17 11:00, Ramana Radhakrishnan wrote: >>>> >>> >>>>> >>> > static bool >>>>> >>> > arm_default_short_enums (void) >>>>> >>> > { >>>>> >>> >- return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; >>>>> >>> >+ return ARM_DEFAULT_SHORT_ENUMS; >>>>> >>> > } >>>>> >>> > >>>>> >>> > >>>>> >>> >diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h >>>>> >>> >index 53cd987..b34bbe8 100644 >>>>> >>> >--- a/gcc/config/arm/rtems.h >>>>> >>> >+++ b/gcc/config/arm/rtems.h >>>>> >>> >@@ -27,3 +27,5 @@ >>>>> >>> > builtin_assert ("system=rtems"); \ >>>>> >>> > TARGET_BPABI_CPP_BUILTINS(); \ >>>>> >>> > } while (0) >>>>> >>> >+ >>>>> >>> >+#define ARM_DEFAULT_SHORT_ENUMS false >>> >> >>> >>It's a change in ABI for the RTEMS platform but it certainly needs a >>> >>documentation update in the release notes . Also, is it necessary that >>> >>you need this in for GCC-7 or can you wait for stage-1 since we are in >>> >>regression fixes mode ? >> > >> > >> >For RTEMS, ABI changes are not really critical. I would like to get this >> >into GCC 6.4. For GCC 7 its not urgent. >> > > The usual policy is not to have ABI changes within sub-releases of a > GCC release cycle. However if the RTEMs community is happy with it, I > have no particular objections. I would however strongly suggest that > if you are fixing it for GCC 6.4 to then fix it for GCC-7*and* > document it in the release notes. For RTEMS ABI changes are not a big deal. Its more important that we don't carry GCC patches around and can use a particular FSF release unmodified. I will add this change to the GCC 6.4 and 7.1 release notes, if this is all right for Jakub.
On 04/04/17 12:50, Sebastian Huber wrote: > On 04/04/17 12:41, Ramana Radhakrishnan wrote: >> [dropping devel at rtems dot org as I don't want more bounces] >> >> On Tue, Apr 4, 2017 at 10:07 AM, Sebastian Huber >> <sebastian.huber@embedded-brains.de> wrote: >>> >On 04/04/17 11:00, Ramana Radhakrishnan wrote: >>>>> >>> >>>>>> >>> > static bool >>>>>> >>> > arm_default_short_enums (void) >>>>>> >>> > { >>>>>> >>> >- return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; >>>>>> >>> >+ return ARM_DEFAULT_SHORT_ENUMS; >>>>>> >>> > } >>>>>> >>> > >>>>>> >>> > >>>>>> >>> >diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h >>>>>> >>> >index 53cd987..b34bbe8 100644 >>>>>> >>> >--- a/gcc/config/arm/rtems.h >>>>>> >>> >+++ b/gcc/config/arm/rtems.h >>>>>> >>> >@@ -27,3 +27,5 @@ >>>>>> >>> > builtin_assert ("system=rtems"); \ >>>>>> >>> > TARGET_BPABI_CPP_BUILTINS(); \ >>>>>> >>> > } while (0) >>>>>> >>> >+ >>>>>> >>> >+#define ARM_DEFAULT_SHORT_ENUMS false >>>> >> >>>> >>It's a change in ABI for the RTEMS platform but it certainly needs a >>>> >>documentation update in the release notes . Also, is it necessary >>>> that >>>> >>you need this in for GCC-7 or can you wait for stage-1 since we >>>> are in >>>> >>regression fixes mode ? >>> > >>> > >>> >For RTEMS, ABI changes are not really critical. I would like to get >>> this >>> >into GCC 6.4. For GCC 7 its not urgent. >>> > >> The usual policy is not to have ABI changes within sub-releases of a >> GCC release cycle. However if the RTEMs community is happy with it, I >> have no particular objections. I would however strongly suggest that >> if you are fixing it for GCC 6.4 to then fix it for GCC-7*and* >> document it in the release notes. > > For RTEMS ABI changes are not a big deal. Its more important that we > don't carry GCC patches around and can use a particular FSF release > unmodified. I will add this change to the GCC 6.4 and 7.1 release > notes, if this is all right for Jakub. v2 of the patch is here: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00138.html
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index b24143e..33d3834 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -26547,11 +26547,15 @@ arm_promote_function_mode (const_tree type ATTRIBUTE_UNUSED, } /* AAPCS based ABIs use short enums by default. */ +#ifndef ARM_DEFAULT_SHORT_ENUMS +#define ARM_DEFAULT_SHORT_ENUMS \ + (TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX) +#endif static bool arm_default_short_enums (void) { - return TARGET_AAPCS_BASED && arm_abi != ARM_ABI_AAPCS_LINUX; + return ARM_DEFAULT_SHORT_ENUMS; } diff --git a/gcc/config/arm/rtems.h b/gcc/config/arm/rtems.h index 53cd987..b34bbe8 100644 --- a/gcc/config/arm/rtems.h +++ b/gcc/config/arm/rtems.h @@ -27,3 +27,5 @@ builtin_assert ("system=rtems"); \ TARGET_BPABI_CPP_BUILTINS(); \ } while (0) + +#define ARM_DEFAULT_SHORT_ENUMS false