* [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m [not found] <54ABACC5.9020406@arm.com> @ 2015-01-06 9:40 ` Mantas Mikaitis 2015-02-03 16:29 ` Richard Earnshaw 0 siblings, 1 reply; 5+ messages in thread From: Mantas Mikaitis @ 2015-01-06 9:40 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2261 bytes --] Ping and changelog spaces removed. Thank you, Mantas M. On 18/11/14 11:58, Richard Earnshaw wrote: > On 18/11/14 11:30, Mantas Mikaitis wrote: >> Incorrect predefinitions for certain target architectures. E.g. arm7-m >> does not contain NEON but the defintion __ARM_NEON_FP was switched on. >> Similarly with armv6 and even armv2. >> >> This patch fixes the predefines for each of the different chips >> containing certain types of the FPU implementations. >> >> Tests: >> >> Tested on arm-none-linux-gnueabi and arm-none-linux-gnueabihf without >> any new regression. >> >> Manually compiled for various targets and all correct definitions were >> present. >> >> Is this patch ok for trunk? >> >> Mantas >> >> gcc/Changelog: >> >> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >> (TARGET_CPU_CPP_BUILTINS): Added second condition before defining __ARM_FP macro. >> >> >> ARM_DEFS.patch >> >> >> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >> index ff4ddac..325fea9 100644 >> --- a/gcc/config/arm/arm.h >> +++ b/gcc/config/arm/arm.h >> @@ -118,7 +118,7 @@ extern char arm_arch_name[]; >> if (TARGET_VFP) \ >> builtin_define ("__VFP_FP__"); \ >> \ >> - if (TARGET_ARM_FP) \ >> + if (TARGET_ARM_FP && !TARGET_SOFT_FLOAT) \ > Wouldn't it be better to factor this into TARGET_ARM_FP? It seems odd > that that macro returns a set of values based on something completely > unavailable for the current compilation. That would also then mirror > the behaviour of TARGET_NEON_FP (see below) and make the internal macros > more consistent. > > R. Thank you. Patch updated. Ok for trunk? Mantas M. gcc/Changelog 2014-12-03 Mantas Mikaits <Mantas.Mikaitis@arm.com> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. (TARGET_ARM_FP): Added !TARGET_SOFT_FLOAT into the conditional definition. gcc/testsuite/ChangeLog: * gcc.target/arm/macro_defs0.c: New test. * gcc.target/arm/macro_defs1.c: New test. * gcc.target/arm/macro_defs2.c: New test. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: mypatch.patch --] [-- Type: text/x-patch; name=mypatch.patch, Size: 2881 bytes --] diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index ff4ddac..7d4cc39 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2343,17 +2343,17 @@ extern int making_const_table; point types. Where bit 1 indicates 16-bit support, bit 2 indicates 32-bit support, bit 3 indicates 64-bit support. */ #define TARGET_ARM_FP \ - (TARGET_VFP_SINGLE ? 4 \ - : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) + (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4 \ + : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \ + : 0) /* Set as a bit mask indicating the available widths of floating point types for hardware NEON floating point. This is the same as TARGET_ARM_FP without the 64-bit bit set. */ -#ifdef TARGET_NEON -#define TARGET_NEON_FP \ - (TARGET_ARM_FP & (0xff ^ 0x08)) -#endif +#define TARGET_NEON_FP \ + (TARGET_NEON ? (TARGET_ARM_FP & (0xff ^ 0x08)) \ + : 0) /* The maximum number of parallel loads or stores we support in an ldm/stm instruction. */ diff --git a/gcc/testsuite/gcc.target/arm/macro_defs0.c b/gcc/testsuite/gcc.target/arm/macro_defs0.c new file mode 100644 index 0000000..198243e --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs0.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-march=*" } {"-march=armv7-m"} } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */ +/* { dg-options "-march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mthumb" } */ + +#ifdef __ARM_FP +#error __ARM_FP should not be defined +#endif + +#ifdef __ARM_NEON_FP +#error __ARM_NEON_FP should not be defined +#endif diff --git a/gcc/testsuite/gcc.target/arm/macro_defs1.c b/gcc/testsuite/gcc.target/arm/macro_defs1.c new file mode 100644 index 0000000..075b71b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-march=*" } { "-march=armv6-m" } } */ +/* { dg-options "-march=armv6-m -mthumb" } */ + +#ifdef __ARM_NEON_FP +#error __ARM_NEON_FP should not be defined +#endif + diff --git a/gcc/testsuite/gcc.target/arm/macro_defs2.c b/gcc/testsuite/gcc.target/arm/macro_defs2.c new file mode 100644 index 0000000..9a96042 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs2.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv7ve -mcpu=cortex-a15 -mfpu=neon-vfpv4" } */ +/* { dg-add-options arm_neon } */ +/* { dg-require-effective-target arm_neon_ok } */ + +#ifndef __ARM_NEON_FP +#error __ARM_NEON_FP is not defined but should be +#endif + +#ifndef __ARM_FP +#error __ARM_FP is not defined but should be +#endif + + ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m 2015-01-06 9:40 ` [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m Mantas Mikaitis @ 2015-02-03 16:29 ` Richard Earnshaw 2015-02-08 15:21 ` Christophe Lyon 0 siblings, 1 reply; 5+ messages in thread From: Richard Earnshaw @ 2015-02-03 16:29 UTC (permalink / raw) To: Mantas Mikaitis, gcc-patches On 06/01/15 09:40, Mantas Mikaitis wrote: > > Ping and changelog spaces removed. > > Thank you, > Mantas M. > > On 18/11/14 11:58, Richard Earnshaw wrote: >> On 18/11/14 11:30, Mantas Mikaitis wrote: >>> Incorrect predefinitions for certain target architectures. E.g. arm7-m >>> does not contain NEON but the defintion __ARM_NEON_FP was switched on. >>> Similarly with armv6 and even armv2. >>> >>> This patch fixes the predefines for each of the different chips >>> containing certain types of the FPU implementations. >>> >>> Tests: >>> >>> Tested on arm-none-linux-gnueabi and arm-none-linux-gnueabihf without >>> any new regression. >>> >>> Manually compiled for various targets and all correct definitions were >>> present. >>> >>> Is this patch ok for trunk? >>> >>> Mantas >>> >>> gcc/Changelog: >>> >>> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >>> (TARGET_CPU_CPP_BUILTINS): Added second condition before defining __ARM_FP macro. >>> >>> >>> ARM_DEFS.patch >>> >>> >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>> index ff4ddac..325fea9 100644 >>> --- a/gcc/config/arm/arm.h >>> +++ b/gcc/config/arm/arm.h >>> @@ -118,7 +118,7 @@ extern char arm_arch_name[]; >>> if (TARGET_VFP) \ >>> builtin_define ("__VFP_FP__"); \ >>> \ >>> - if (TARGET_ARM_FP) \ >>> + if (TARGET_ARM_FP && !TARGET_SOFT_FLOAT) \ >> Wouldn't it be better to factor this into TARGET_ARM_FP? It seems odd >> that that macro returns a set of values based on something completely >> unavailable for the current compilation. That would also then mirror >> the behaviour of TARGET_NEON_FP (see below) and make the internal macros >> more consistent. >> >> R. > > Thank you. Patch updated. > > Ok for trunk? > > Mantas M. > > gcc/Changelog > > 2014-12-03 Mantas Mikaits <Mantas.Mikaitis@arm.com> > > * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. > (TARGET_ARM_FP): Added !TARGET_SOFT_FLOAT into the conditional definition. > > gcc/testsuite/ChangeLog: > > * gcc.target/arm/macro_defs0.c: New test. > * gcc.target/arm/macro_defs1.c: New test. > * gcc.target/arm/macro_defs2.c: New test. > > OK. However, watch your ChangeLog line length (80 char limit). Also, entries (even continuation lines) should be indented with exactly one (hard) tab. R. > > > > > > > > > > > > > mypatch.patch > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > index ff4ddac..7d4cc39 100644 > --- a/gcc/config/arm/arm.h > +++ b/gcc/config/arm/arm.h > @@ -2343,17 +2343,17 @@ extern int making_const_table; > point types. Where bit 1 indicates 16-bit support, bit 2 indicates > 32-bit support, bit 3 indicates 64-bit support. */ > #define TARGET_ARM_FP \ > - (TARGET_VFP_SINGLE ? 4 \ > - : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) > + (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4 \ > + : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \ > + : 0) > > > /* Set as a bit mask indicating the available widths of floating point > types for hardware NEON floating point. This is the same as > TARGET_ARM_FP without the 64-bit bit set. */ > -#ifdef TARGET_NEON > -#define TARGET_NEON_FP \ > - (TARGET_ARM_FP & (0xff ^ 0x08)) > -#endif > +#define TARGET_NEON_FP \ > + (TARGET_NEON ? (TARGET_ARM_FP & (0xff ^ 0x08)) \ > + : 0) > > /* The maximum number of parallel loads or stores we support in an ldm/stm > instruction. */ > diff --git a/gcc/testsuite/gcc.target/arm/macro_defs0.c b/gcc/testsuite/gcc.target/arm/macro_defs0.c > new file mode 100644 > index 0000000..198243e > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/macro_defs0.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-skip-if "avoid conflicting multilib options" > + { *-*-* } { "-march=*" } {"-march=armv7-m"} } */ > +/* { dg-skip-if "avoid conflicting multilib options" > + { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */ > +/* { dg-options "-march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mthumb" } */ > + > +#ifdef __ARM_FP > +#error __ARM_FP should not be defined > +#endif > + > +#ifdef __ARM_NEON_FP > +#error __ARM_NEON_FP should not be defined > +#endif > diff --git a/gcc/testsuite/gcc.target/arm/macro_defs1.c b/gcc/testsuite/gcc.target/arm/macro_defs1.c > new file mode 100644 > index 0000000..075b71b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/macro_defs1.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile } */ > +/* { dg-skip-if "avoid conflicting multilib options" > + { *-*-* } { "-march=*" } { "-march=armv6-m" } } */ > +/* { dg-options "-march=armv6-m -mthumb" } */ > + > +#ifdef __ARM_NEON_FP > +#error __ARM_NEON_FP should not be defined > +#endif > + > diff --git a/gcc/testsuite/gcc.target/arm/macro_defs2.c b/gcc/testsuite/gcc.target/arm/macro_defs2.c > new file mode 100644 > index 0000000..9a96042 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arm/macro_defs2.c > @@ -0,0 +1,14 @@ > +/* { dg-do compile } */ > +/* { dg-options "-march=armv7ve -mcpu=cortex-a15 -mfpu=neon-vfpv4" } */ > +/* { dg-add-options arm_neon } */ > +/* { dg-require-effective-target arm_neon_ok } */ > + > +#ifndef __ARM_NEON_FP > +#error __ARM_NEON_FP is not defined but should be > +#endif > + > +#ifndef __ARM_FP > +#error __ARM_FP is not defined but should be > +#endif > + > + > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m 2015-02-03 16:29 ` Richard Earnshaw @ 2015-02-08 15:21 ` Christophe Lyon 2015-02-13 9:33 ` Mantas Mikaitis 0 siblings, 1 reply; 5+ messages in thread From: Christophe Lyon @ 2015-02-08 15:21 UTC (permalink / raw) To: Richard Earnshaw; +Cc: Mantas Mikaitis, gcc-patches On 3 February 2015 at 17:29, Richard Earnshaw <rearnsha@arm.com> wrote: > On 06/01/15 09:40, Mantas Mikaitis wrote: >> >> Ping and changelog spaces removed. >> >> Thank you, >> Mantas M. >> >> On 18/11/14 11:58, Richard Earnshaw wrote: >>> On 18/11/14 11:30, Mantas Mikaitis wrote: >>>> Incorrect predefinitions for certain target architectures. E.g. arm7-m >>>> does not contain NEON but the defintion __ARM_NEON_FP was switched on. >>>> Similarly with armv6 and even armv2. >>>> >>>> This patch fixes the predefines for each of the different chips >>>> containing certain types of the FPU implementations. >>>> >>>> Tests: >>>> >>>> Tested on arm-none-linux-gnueabi and arm-none-linux-gnueabihf without >>>> any new regression. >>>> >>>> Manually compiled for various targets and all correct definitions were >>>> present. >>>> >>>> Is this patch ok for trunk? >>>> >>>> Mantas >>>> >>>> gcc/Changelog: >>>> >>>> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >>>> (TARGET_CPU_CPP_BUILTINS): Added second condition before defining __ARM_FP macro. >>>> >>>> >>>> ARM_DEFS.patch >>>> >>>> >>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>>> index ff4ddac..325fea9 100644 >>>> --- a/gcc/config/arm/arm.h >>>> +++ b/gcc/config/arm/arm.h >>>> @@ -118,7 +118,7 @@ extern char arm_arch_name[]; >>>> if (TARGET_VFP) \ >>>> builtin_define ("__VFP_FP__"); \ >>>> \ >>>> - if (TARGET_ARM_FP) \ >>>> + if (TARGET_ARM_FP && !TARGET_SOFT_FLOAT) \ >>> Wouldn't it be better to factor this into TARGET_ARM_FP? It seems odd >>> that that macro returns a set of values based on something completely >>> unavailable for the current compilation. That would also then mirror >>> the behaviour of TARGET_NEON_FP (see below) and make the internal macros >>> more consistent. >>> >>> R. >> >> Thank you. Patch updated. >> >> Ok for trunk? >> >> Mantas M. >> >> gcc/Changelog >> >> 2014-12-03 Mantas Mikaits <Mantas.Mikaitis@arm.com> >> >> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >> (TARGET_ARM_FP): Added !TARGET_SOFT_FLOAT into the conditional definition. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/arm/macro_defs0.c: New test. >> * gcc.target/arm/macro_defs1.c: New test. >> * gcc.target/arm/macro_defs2.c: New test. >> >> > > OK. However, watch your ChangeLog line length (80 char limit). Also, > entries (even continuation lines) should be indented with exactly one > (hard) tab. > > R. > >> >> >> >> >> >> >> >> >> >> >> >> >> mypatch.patch >> >> >> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >> index ff4ddac..7d4cc39 100644 >> --- a/gcc/config/arm/arm.h >> +++ b/gcc/config/arm/arm.h >> @@ -2343,17 +2343,17 @@ extern int making_const_table; >> point types. Where bit 1 indicates 16-bit support, bit 2 indicates >> 32-bit support, bit 3 indicates 64-bit support. */ >> #define TARGET_ARM_FP \ >> - (TARGET_VFP_SINGLE ? 4 \ >> - : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) >> + (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4 \ >> + : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \ >> + : 0) >> >> >> /* Set as a bit mask indicating the available widths of floating point >> types for hardware NEON floating point. This is the same as >> TARGET_ARM_FP without the 64-bit bit set. */ >> -#ifdef TARGET_NEON >> -#define TARGET_NEON_FP \ >> - (TARGET_ARM_FP & (0xff ^ 0x08)) >> -#endif >> +#define TARGET_NEON_FP \ >> + (TARGET_NEON ? (TARGET_ARM_FP & (0xff ^ 0x08)) \ >> + : 0) >> >> /* The maximum number of parallel loads or stores we support in an ldm/stm >> instruction. */ >> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs0.c b/gcc/testsuite/gcc.target/arm/macro_defs0.c >> new file mode 100644 >> index 0000000..198243e >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/macro_defs0.c >> @@ -0,0 +1,14 @@ >> +/* { dg-do compile } */ >> +/* { dg-skip-if "avoid conflicting multilib options" >> + { *-*-* } { "-march=*" } {"-march=armv7-m"} } */ >> +/* { dg-skip-if "avoid conflicting multilib options" >> + { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */ >> +/* { dg-options "-march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mthumb" } */ >> + >> +#ifdef __ARM_FP >> +#error __ARM_FP should not be defined >> +#endif >> + >> +#ifdef __ARM_NEON_FP >> +#error __ARM_NEON_FP should not be defined >> +#endif >> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs1.c b/gcc/testsuite/gcc.target/arm/macro_defs1.c >> new file mode 100644 >> index 0000000..075b71b >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/macro_defs1.c >> @@ -0,0 +1,9 @@ >> +/* { dg-do compile } */ >> +/* { dg-skip-if "avoid conflicting multilib options" >> + { *-*-* } { "-march=*" } { "-march=armv6-m" } } */ >> +/* { dg-options "-march=armv6-m -mthumb" } */ Minor comment: You are probably aware of that, but in a multilib which forces '-marm' this test fails because: testsuite/gcc.target/arm/macro_defs1.c:1:0: error: target CPU does not support ARM mode (as a result of compiling it with -march=armv6-m -mthumb -marm) Christophe >> + >> +#ifdef __ARM_NEON_FP >> +#error __ARM_NEON_FP should not be defined >> +#endif >> + >> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs2.c b/gcc/testsuite/gcc.target/arm/macro_defs2.c >> new file mode 100644 >> index 0000000..9a96042 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/arm/macro_defs2.c >> @@ -0,0 +1,14 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-march=armv7ve -mcpu=cortex-a15 -mfpu=neon-vfpv4" } */ >> +/* { dg-add-options arm_neon } */ >> +/* { dg-require-effective-target arm_neon_ok } */ >> + >> +#ifndef __ARM_NEON_FP >> +#error __ARM_NEON_FP is not defined but should be >> +#endif >> + >> +#ifndef __ARM_FP >> +#error __ARM_FP is not defined but should be >> +#endif >> + >> + >> > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m 2015-02-08 15:21 ` Christophe Lyon @ 2015-02-13 9:33 ` Mantas Mikaitis 0 siblings, 0 replies; 5+ messages in thread From: Mantas Mikaitis @ 2015-02-13 9:33 UTC (permalink / raw) To: Christophe Lyon, Richard Earnshaw; +Cc: gcc-patches On 08/02/15 15:20, Christophe Lyon wrote: > On 3 February 2015 at 17:29, Richard Earnshaw <rearnsha@arm.com> wrote: >> On 06/01/15 09:40, Mantas Mikaitis wrote: >>> Ping and changelog spaces removed. >>> >>> Thank you, >>> Mantas M. >>> >>> On 18/11/14 11:58, Richard Earnshaw wrote: >>>> On 18/11/14 11:30, Mantas Mikaitis wrote: >>>>> Incorrect predefinitions for certain target architectures. E.g. arm7-m >>>>> does not contain NEON but the defintion __ARM_NEON_FP was switched on. >>>>> Similarly with armv6 and even armv2. >>>>> >>>>> This patch fixes the predefines for each of the different chips >>>>> containing certain types of the FPU implementations. >>>>> >>>>> Tests: >>>>> >>>>> Tested on arm-none-linux-gnueabi and arm-none-linux-gnueabihf without >>>>> any new regression. >>>>> >>>>> Manually compiled for various targets and all correct definitions were >>>>> present. >>>>> >>>>> Is this patch ok for trunk? >>>>> >>>>> Mantas >>>>> >>>>> gcc/Changelog: >>>>> >>>>> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >>>>> (TARGET_CPU_CPP_BUILTINS): Added second condition before defining __ARM_FP macro. >>>>> >>>>> >>>>> ARM_DEFS.patch >>>>> >>>>> >>>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>>>> index ff4ddac..325fea9 100644 >>>>> --- a/gcc/config/arm/arm.h >>>>> +++ b/gcc/config/arm/arm.h >>>>> @@ -118,7 +118,7 @@ extern char arm_arch_name[]; >>>>> if (TARGET_VFP) \ >>>>> builtin_define ("__VFP_FP__"); \ >>>>> \ >>>>> - if (TARGET_ARM_FP) \ >>>>> + if (TARGET_ARM_FP && !TARGET_SOFT_FLOAT) \ >>>> Wouldn't it be better to factor this into TARGET_ARM_FP? It seems odd >>>> that that macro returns a set of values based on something completely >>>> unavailable for the current compilation. That would also then mirror >>>> the behaviour of TARGET_NEON_FP (see below) and make the internal macros >>>> more consistent. >>>> >>>> R. >>> Thank you. Patch updated. >>> >>> Ok for trunk? >>> >>> Mantas M. >>> >>> gcc/Changelog >>> >>> 2014-12-03 Mantas Mikaits <Mantas.Mikaitis@arm.com> >>> >>> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >>> (TARGET_ARM_FP): Added !TARGET_SOFT_FLOAT into the conditional definition. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * gcc.target/arm/macro_defs0.c: New test. >>> * gcc.target/arm/macro_defs1.c: New test. >>> * gcc.target/arm/macro_defs2.c: New test. >>> >>> >> OK. However, watch your ChangeLog line length (80 char limit). Also, >> entries (even continuation lines) should be indented with exactly one >> (hard) tab. >> >> R. >> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> >>> mypatch.patch >>> >>> >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >>> index ff4ddac..7d4cc39 100644 >>> --- a/gcc/config/arm/arm.h >>> +++ b/gcc/config/arm/arm.h >>> @@ -2343,17 +2343,17 @@ extern int making_const_table; >>> point types. Where bit 1 indicates 16-bit support, bit 2 indicates >>> 32-bit support, bit 3 indicates 64-bit support. */ >>> #define TARGET_ARM_FP \ >>> - (TARGET_VFP_SINGLE ? 4 \ >>> - : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) >>> + (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4 \ >>> + : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \ >>> + : 0) >>> >>> >>> /* Set as a bit mask indicating the available widths of floating point >>> types for hardware NEON floating point. This is the same as >>> TARGET_ARM_FP without the 64-bit bit set. */ >>> -#ifdef TARGET_NEON >>> -#define TARGET_NEON_FP \ >>> - (TARGET_ARM_FP & (0xff ^ 0x08)) >>> -#endif >>> +#define TARGET_NEON_FP \ >>> + (TARGET_NEON ? (TARGET_ARM_FP & (0xff ^ 0x08)) \ >>> + : 0) >>> >>> /* The maximum number of parallel loads or stores we support in an ldm/stm >>> instruction. */ >>> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs0.c b/gcc/testsuite/gcc.target/arm/macro_defs0.c >>> new file mode 100644 >>> index 0000000..198243e >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/arm/macro_defs0.c >>> @@ -0,0 +1,14 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-skip-if "avoid conflicting multilib options" >>> + { *-*-* } { "-march=*" } {"-march=armv7-m"} } */ >>> +/* { dg-skip-if "avoid conflicting multilib options" >>> + { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */ >>> +/* { dg-options "-march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mthumb" } */ >>> + >>> +#ifdef __ARM_FP >>> +#error __ARM_FP should not be defined >>> +#endif >>> + >>> +#ifdef __ARM_NEON_FP >>> +#error __ARM_NEON_FP should not be defined >>> +#endif >>> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs1.c b/gcc/testsuite/gcc.target/arm/macro_defs1.c >>> new file mode 100644 >>> index 0000000..075b71b >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/arm/macro_defs1.c >>> @@ -0,0 +1,9 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-skip-if "avoid conflicting multilib options" >>> + { *-*-* } { "-march=*" } { "-march=armv6-m" } } */ >>> +/* { dg-options "-march=armv6-m -mthumb" } */ > Minor comment: > You are probably aware of that, but in a multilib which forces '-marm' > this test fails because: > testsuite/gcc.target/arm/macro_defs1.c:1:0: error: target CPU does not > support ARM mode > (as a result of compiling it with -march=armv6-m -mthumb -marm) > > Christophe Hi Christophe, Good spot, thank you. I will generate a fix for this. Best regards, -- Mantas M. >>> + >>> +#ifdef __ARM_NEON_FP >>> +#error __ARM_NEON_FP should not be defined >>> +#endif >>> + >>> diff --git a/gcc/testsuite/gcc.target/arm/macro_defs2.c b/gcc/testsuite/gcc.target/arm/macro_defs2.c >>> new file mode 100644 >>> index 0000000..9a96042 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/arm/macro_defs2.c >>> @@ -0,0 +1,14 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-options "-march=armv7ve -mcpu=cortex-a15 -mfpu=neon-vfpv4" } */ >>> +/* { dg-add-options arm_neon } */ >>> +/* { dg-require-effective-target arm_neon_ok } */ >>> + >>> +#ifndef __ARM_NEON_FP >>> +#error __ARM_NEON_FP is not defined but should be >>> +#endif >>> + >>> +#ifndef __ARM_FP >>> +#error __ARM_FP is not defined but should be >>> +#endif >>> + >>> + >>> >> ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <54ABAB4D.2010809@arm.com>]
* [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m [not found] <54ABAB4D.2010809@arm.com> @ 2015-01-06 9:32 ` Mantas Mikaitis 0 siblings, 0 replies; 5+ messages in thread From: Mantas Mikaitis @ 2015-01-06 9:32 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2193 bytes --] Ping. Thank you, Mantas M. On 18/11/14 11:58, Richard Earnshaw wrote: > On 18/11/14 11:30, Mantas Mikaitis wrote: >> Incorrect predefinitions for certain target architectures. E.g. arm7-m >> does not contain NEON but the defintion __ARM_NEON_FP was switched on. >> Similarly with armv6 and even armv2. >> >> This patch fixes the predefines for each of the different chips >> containing certain types of the FPU implementations. >> >> Tests: >> >> Tested on arm-none-linux-gnueabi and arm-none-linux-gnueabihf without >> any new regression. >> >> Manually compiled for various targets and all correct definitions were >> present. >> >> Is this patch ok for trunk? >> >> Mantas >> >> gcc/Changelog: >> >> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. >> (TARGET_CPU_CPP_BUILTINS): Added second condition before defining __ARM_FP macro. >> >> >> ARM_DEFS.patch >> >> >> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h >> index ff4ddac..325fea9 100644 >> --- a/gcc/config/arm/arm.h >> +++ b/gcc/config/arm/arm.h >> @@ -118,7 +118,7 @@ extern char arm_arch_name[]; >> if (TARGET_VFP) \ >> builtin_define ("__VFP_FP__"); \ >> \ >> - if (TARGET_ARM_FP) \ >> + if (TARGET_ARM_FP && !TARGET_SOFT_FLOAT) \ > Wouldn't it be better to factor this into TARGET_ARM_FP? It seems odd > that that macro returns a set of values based on something completely > unavailable for the current compilation. That would also then mirror > the behaviour of TARGET_NEON_FP (see below) and make the internal macros > more consistent. > > R. Thank you. Patch updated. Ok for trunk? Mantas M. gcc/Changelog 2014-12-03 Mantas Mikaits <Mantas.Mikaitis@arm.com> * config/arm/arm.h (TARGET_NEON_FP): Removed conditional definition, define to zero if !TARGET_NEON. (TARGET_ARM_FP): Added !TARGET_SOFT_FLOAT into the conditional definition. gcc/testsuite/ChangeLog: * gcc.target/arm/macro_defs0.c: New test. * gcc.target/arm/macro_defs1.c: New test. * gcc.target/arm/macro_defs2.c: New test. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: mypatch.patch --] [-- Type: text/x-patch; name=mypatch.patch, Size: 2875 bytes --] diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index ff4ddac..7d4cc39 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2343,17 +2343,17 @@ extern int making_const_table; point types. Where bit 1 indicates 16-bit support, bit 2 indicates 32-bit support, bit 3 indicates 64-bit support. */ #define TARGET_ARM_FP \ - (TARGET_VFP_SINGLE ? 4 \ - : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) + (!TARGET_SOFT_FLOAT ? (TARGET_VFP_SINGLE ? 4 \ + : (TARGET_VFP_DOUBLE ? (TARGET_FP16 ? 14 : 12) : 0)) \ + : 0) /* Set as a bit mask indicating the available widths of floating point types for hardware NEON floating point. This is the same as TARGET_ARM_FP without the 64-bit bit set. */ -#ifdef TARGET_NEON -#define TARGET_NEON_FP \ - (TARGET_ARM_FP & (0xff ^ 0x08)) -#endif +#define TARGET_NEON_FP \ + (TARGET_NEON ? (TARGET_ARM_FP & (0xff ^ 0x08)) \ + : 0) /* The maximum number of parallel loads or stores we support in an ldm/stm instruction. */ diff --git a/gcc/testsuite/gcc.target/arm/macro_defs0.c b/gcc/testsuite/gcc.target/arm/macro_defs0.c new file mode 100644 index 0000000..198243e --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs0.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-march=*" } {"-march=armv7-m"} } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-mfloat-abi=*" } { "-mfloat-abi=soft" } } */ +/* { dg-options "-march=armv7-m -mcpu=cortex-m3 -mfloat-abi=soft -mthumb" } */ + +#ifdef __ARM_FP +#error __ARM_FP should not be defined +#endif + +#ifdef __ARM_NEON_FP +#error __ARM_NEON_FP should not be defined +#endif diff --git a/gcc/testsuite/gcc.target/arm/macro_defs1.c b/gcc/testsuite/gcc.target/arm/macro_defs1.c new file mode 100644 index 0000000..075b71b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs1.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-skip-if "avoid conflicting multilib options" + { *-*-* } { "-march=*" } { "-march=armv6-m" } } */ +/* { dg-options "-march=armv6-m -mthumb" } */ + +#ifdef __ARM_NEON_FP +#error __ARM_NEON_FP should not be defined +#endif + diff --git a/gcc/testsuite/gcc.target/arm/macro_defs2.c b/gcc/testsuite/gcc.target/arm/macro_defs2.c new file mode 100644 index 0000000..9a96042 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/macro_defs2.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options "-march=armv7ve -mcpu=cortex-a15 -mfpu=neon-vfpv4" } */ +/* { dg-add-options arm_neon } */ +/* { dg-require-effective-target arm_neon_ok } */ + +#ifndef __ARM_NEON_FP +#error __ARM_NEON_FP is not defined but should be +#endif + +#ifndef __ARM_FP +#error __ARM_FP is not defined but should be +#endif + + ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-13 9:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <54ABACC5.9020406@arm.com>
2015-01-06 9:40 ` [PATCH][ARM][PING] __ARM_FP & __ARM_NEON_FP defined when -march=armv7-m Mantas Mikaitis
2015-02-03 16:29 ` Richard Earnshaw
2015-02-08 15:21 ` Christophe Lyon
2015-02-13 9:33 ` Mantas Mikaitis
[not found] <54ABAB4D.2010809@arm.com>
2015-01-06 9:32 ` Mantas Mikaitis
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).