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