[ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros

as discussed with Kyrill 
(https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00307.html), this patch 
avoids confusing (for the testsuite) macro redefinition warning or 
pedantic errors when the user changes FP versions implicitly with a 
#pragma GCC target. The warning is kept when the macro is redefined 
explicitly by the user.

tested on arm-linux-gnueabi for {,-mfpu=neon-fp-armv8,-mfpu=neon}

Hi Christian,

On 07/01/16 15:40, Christian Bruel wrote:
> as discussed with Kyrill (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00307.html), this patch avoids confusing (for the testsuite) macro redefinition warning or pedantic errors when the user changes FP versions implicitly with a #pragma 
> GCC target. The warning is kept when the macro is redefined explicitly by the user.
>
> tested on arm-linux-gnueabi for {,-mfpu=neon-fp-armv8,-mfpu=neon}
>
>

> Index: config/arm/arm-c.c
> ===================================================================
> --- config/arm/arm-c.c	(revision 232101)
> +++ config/arm/arm-c.c	(working copy)
> @@ -23,6 +23,7 @@
>   #include "c-family/c-common.h"
>   #include "tm_p.h"
>   #include "c-family/c-pragma.h"
> +#include "stringpool.h"
>   
>   /* Output C specific EABI object attributes.  These can not be done in
>      arm.c because they require information from the C frontend.  */
> @@ -245,8 +246,18 @@ arm_pragma_target_parse (tree args, tree
>   
>         /* Update macros.  */
>         gcc_assert (cur_opt->x_target_flags == target_flags);
> -      /* This one can be redefined by the pragma without warning.  */
> -      cpp_undef (parse_in, "__ARM_FP");
> +
> +      /* Don't warn for macros that have context sensitive values depending on
> +	 other attributes.
> +	 See warn_of_redefinition, Reset after cpp_create_definition.  */
> +      tree acond_macro = get_identifier ("__ARM_NEON_FP");
> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ;
> +
> +      acond_macro = get_identifier ("__ARM_FP");
> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL;
> +
> +      acond_macro = get_identifier ("__ARM_FEATURE_LDREX");
> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL;

I see this mechanism also being used by rs6000, s390 and spu but I'm not very familiar with it.
Could you please provide a short explanatino of what NODE_CONDITIONAL means?
I suspec this is ok, but I'd like to get a better understanding of what's going on here.

Thanks,
Kyrill

Hi Kyrill,

On 01/11/2016 12:32 PM, Kyrill Tkachov wrote:
> Hi Christian,
>
> On 07/01/16 15:40, Christian Bruel wrote:
>> as discussed with Kyrill (https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00307.html), this patch avoids confusing (for the testsuite) macro redefinition warning or pedantic errors when the user changes FP versions implicitly with a #pragma
>> GCC target. The warning is kept when the macro is redefined explicitly by the user.
>>
>> tested on arm-linux-gnueabi for {,-mfpu=neon-fp-armv8,-mfpu=neon}
>>
>>
>> Index: config/arm/arm-c.c
>> ===================================================================
>> --- config/arm/arm-c.c	(revision 232101)
>> +++ config/arm/arm-c.c	(working copy)
>> @@ -23,6 +23,7 @@
>>    #include "c-family/c-common.h"
>>    #include "tm_p.h"
>>    #include "c-family/c-pragma.h"
>> +#include "stringpool.h"
>>
>>    /* Output C specific EABI object attributes.  These can not be done in
>>       arm.c because they require information from the C frontend.  */
>> @@ -245,8 +246,18 @@ arm_pragma_target_parse (tree args, tree
>>
>>          /* Update macros.  */
>>          gcc_assert (cur_opt->x_target_flags == target_flags);
>> -      /* This one can be redefined by the pragma without warning.  */
>> -      cpp_undef (parse_in, "__ARM_FP");
>> +
>> +      /* Don't warn for macros that have context sensitive values depending on
>> +	 other attributes.
>> +	 See warn_of_redefinition, Reset after cpp_create_definition.  */
>> +      tree acond_macro = get_identifier ("__ARM_NEON_FP");
>> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ;
>> +
>> +      acond_macro = get_identifier ("__ARM_FP");
>> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL;
>> +
>> +      acond_macro = get_identifier ("__ARM_FEATURE_LDREX");
>> +      C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL;
> I see this mechanism also being used by rs6000, s390 and spu but I'm not very familiar with it.
> Could you please provide a short explanatino of what NODE_CONDITIONAL means?
> I suspec this is ok, but I'd like to get a better understanding of what's going on here.

This is part of a larger support for context-sensitive keywords 
implemented for rs6000 (patch digging 
https://gcc.gnu.org/ml/gcc-patches/2007-12/msg00306.html).

On ARM those preprocessor macros are always defined so we don't need to 
define the macro_to_expand cpp hook.  However their value does 
legitimately change in the specific #pragma target path so we reuse this 
logic for this path.
The macro will always be correctly recognized on the other 
paths(#ifdef,...) because the NODE_CONDITIONAL bit is cleared when 
defined (see cpp_create_definition). The idea of the original rs6000 
patch is that if a macro is user-defined it is not context-sensitive.
So this is absolutely a reuse of a subpart of a larger support, but this 
logic fits and works well for our goal, given that the preprocessor 
value can change between target contexts, and that the bit is not set 
for "normal" builtin definition.

In short:  Ask `warn_of_redefinition` to be permissive about those macro 
redefinitions when we come from a pragma target definition, as if we 
were redefining a context-sensitive macro,  the difference is that it is 
always defined.

does this sound clear :-) ?

Cheers


> Thanks,
> Kyrill