On 3 February 2015 at 17:29, Richard Earnshaw <rearn...@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.mikai...@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
>> +
>> +
>>
>
>
