* [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
@ 2016-01-07 15:41 Christian Bruel
2016-01-11 11:32 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Christian Bruel @ 2016-01-07 15:41 UTC (permalink / raw)
To: kyrylo.tkachov; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 397 bytes --]
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}
[-- Attachment #2: pr69180.patch --]
[-- Type: text/x-patch, Size: 2444 bytes --]
2016-01-06 Christian Bruel <christian.bruel@st.com>
PR target/69180
* config/arm/arm-c.c (arm_pragma_target_parse): Set NODE_CONDITIONAL
for __ARM_NEON_FP, __ARM_FP, _ARM_FEATURE_LDREX.
2016-01-06 Christian Bruel <christian.bruel@st.com>
PR target/69180
* gcc.target/arm/pr69180.c: New test.
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;
arm_cpu_builtins (parse_in);
Index: testsuite/gcc.target/arm/pr69180.c
===================================================================
--- testsuite/gcc.target/arm/pr69180.c (revision 0)
+++ testsuite/gcc.target/arm/pr69180.c (working copy)
@@ -0,0 +1,16 @@
+/* PR target/69180
+ Check that __ARM_NEON_FP redefinition warns for user setting and not for
+ #pragma GCC target. */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-mfloat-abi=softfp -mfpu=neon" } */
+
+#pragma GCC target ("fpu=neon-fp-armv8")
+
+#define __ARM_NEON_FP 0
+#define __ARM_FP 0
+#define __ARM_FEATURE_LDREX 0
+
+/* { dg-warning ".__ARM_NEON_FP. redefined" "" { target *-*-* } 10 } */
+/* { dg-warning ".__ARM_FP. redefined" "" { target *-*-* } 11 } */
+/* { dg-warning ".__ARM_FEATURE_LDREX. redefined" "" { target *-*-* } 12 } */
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
2016-01-07 15:41 [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros Christian Bruel
@ 2016-01-11 11:32 ` Kyrill Tkachov
2016-01-11 12:54 ` Christian Bruel
0 siblings, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2016-01-11 11:32 UTC (permalink / raw)
To: Christian Bruel; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
2016-01-11 11:32 ` Kyrill Tkachov
@ 2016-01-11 12:54 ` Christian Bruel
2016-01-11 14:37 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Christian Bruel @ 2016-01-11 12:54 UTC (permalink / raw)
To: Kyrill Tkachov; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
2016-01-11 12:54 ` Christian Bruel
@ 2016-01-11 14:37 ` Kyrill Tkachov
2016-01-12 9:01 ` Christian Bruel
0 siblings, 1 reply; 6+ messages in thread
From: Kyrill Tkachov @ 2016-01-11 14:37 UTC (permalink / raw)
To: Christian Bruel; +Cc: gcc-patches
On 11/01/16 12:54, Christian Bruel wrote:
> 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 :-) ?
>
Thanks, it's much clearer now.
A couple of comments on the patch then
+ tree acond_macro = get_identifier ("__ARM_NEON_FP");
+ C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ;
So what happens if __ARM_FP was never defined, does get_identifier return NULL_TREE?
If so, won't C_CPP_HASHNODE (acond_macro)->flags ICE?
Index: testsuite/gcc.target/arm/pr69180.c
===================================================================
--- testsuite/gcc.target/arm/pr69180.c (revision 0)
+++ testsuite/gcc.target/arm/pr69180.c (working copy)
@@ -0,0 +1,16 @@
+/* PR target/69180
+ Check that __ARM_NEON_FP redefinition warns for user setting and not for
+ #pragma GCC target. */
+/* { dg-do compile } */
+/* { dg-require-effective-target arm_neon_ok } */
+/* { dg-options "-mfloat-abi=softfp -mfpu=neon" } */
+
I believe we should use /* { dg-add-options arm_neon } */ here.
Thanks,
Kyrill
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
2016-01-11 14:37 ` Kyrill Tkachov
@ 2016-01-12 9:01 ` Christian Bruel
2016-01-12 9:24 ` Kyrill Tkachov
0 siblings, 1 reply; 6+ messages in thread
From: Christian Bruel @ 2016-01-12 9:01 UTC (permalink / raw)
To: Kyrill Tkachov; +Cc: gcc-patches
On 01/11/2016 03:37 PM, Kyrill Tkachov wrote:
> On 11/01/16 12:54, Christian Bruel wrote:
>> 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 :-) ?
>>
> Thanks, it's much clearer now.
> A couple of comments on the patch then
>
> + tree acond_macro = get_identifier ("__ARM_NEON_FP");
> + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ;
>
> So what happens if __ARM_FP was never defined, does get_identifier return NULL_TREE?
> If so, won't C_CPP_HASHNODE (acond_macro)->flags ICE?
get_identifier returns an allocated tree, even in not in the pool already. So won't ICE.
>
> Index: testsuite/gcc.target/arm/pr69180.c
> ===================================================================
> --- testsuite/gcc.target/arm/pr69180.c (revision 0)
> +++ testsuite/gcc.target/arm/pr69180.c (working copy)
> @@ -0,0 +1,16 @@
> +/* PR target/69180
> + Check that __ARM_NEON_FP redefinition warns for user setting and not for
> + #pragma GCC target. */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_neon_ok } */
> +/* { dg-options "-mfloat-abi=softfp -mfpu=neon" } */
> +
>
> I believe we should use /* { dg-add-options arm_neon } */ here.
I also first did this, but the test would fail because -pedantic-error set by DEFAULT_CFLAGS turns the warning into errors. So I preferred to reset explicitly the options.
>
> Thanks,
> Kyrill
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros
2016-01-12 9:01 ` Christian Bruel
@ 2016-01-12 9:24 ` Kyrill Tkachov
0 siblings, 0 replies; 6+ messages in thread
From: Kyrill Tkachov @ 2016-01-12 9:24 UTC (permalink / raw)
To: Christian Bruel; +Cc: gcc-patches
On 12/01/16 09:00, Christian Bruel wrote:
>
>
> On 01/11/2016 03:37 PM, Kyrill Tkachov wrote:
>> On 11/01/16 12:54, Christian Bruel wrote:
>>> 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 :-) ?
>>>
>> Thanks, it's much clearer now.
>> A couple of comments on the patch then
>>
>> + tree acond_macro = get_identifier ("__ARM_NEON_FP");
>> + C_CPP_HASHNODE (acond_macro)->flags |= NODE_CONDITIONAL ;
>>
>> So what happens if __ARM_FP was never defined, does get_identifier return NULL_TREE?
>> If so, won't C_CPP_HASHNODE (acond_macro)->flags ICE?
>
> get_identifier returns an allocated tree, even in not in the pool already. So won't ICE.
>
>>
>> Index: testsuite/gcc.target/arm/pr69180.c
>> ===================================================================
>> --- testsuite/gcc.target/arm/pr69180.c (revision 0)
>> +++ testsuite/gcc.target/arm/pr69180.c (working copy)
>> @@ -0,0 +1,16 @@
>> +/* PR target/69180
>> + Check that __ARM_NEON_FP redefinition warns for user setting and not for
>> + #pragma GCC target. */
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target arm_neon_ok } */
>> +/* { dg-options "-mfloat-abi=softfp -mfpu=neon" } */
>> +
>>
>> I believe we should use /* { dg-add-options arm_neon } */ here.
>
> I also first did this, but the test would fail because -pedantic-error set by DEFAULT_CFLAGS turns the warning into errors. So I preferred to reset explicitly the options.
>
Thanks for the explanations.
This is ok for trunk.
Kyrill
>>
>> Thanks,
>> Kyrill
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-12 9:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07 15:41 [PATCH, ARM] Fox target/69180] #pragma GCC target should not warn about redefined macros Christian Bruel
2016-01-11 11:32 ` Kyrill Tkachov
2016-01-11 12:54 ` Christian Bruel
2016-01-11 14:37 ` Kyrill Tkachov
2016-01-12 9:01 ` Christian Bruel
2016-01-12 9:24 ` Kyrill Tkachov
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).