diff mbox

[ARM] Fix redefinition of cpp macros with #pragma GCC pop,reset

Message ID 56C46CB0.5030500@st.com
State New
Headers show

Commit Message

Christian Bruel Feb. 17, 2016, 12:50 p.m. UTC 
target_option_current_node, used in c-pragma.c to check if a state
should be popped or reseted to the previous value, was not set when
switching state with #pragma GCC target (I missed to see that, since it
is done for pop,reset). So in some cases the state might not be reset
correctly.

This patch sets it for #pragma GCC target paths and update the comments
as well to clarify this point.

As a benefit we now use this cached value instead of 
build_target_option_node (&global_options), this should speed up (a
little bit) this path when processing arm_neon.h.

One effect of it is that some predicate tests (e.g arm_neonv2_ok) in the
testsuite was returning the wrong value, thus marking some test as
UNRESOLVED instead of PASS. See the reduced case of the issue attached
is the patch.

Regtested, a few new PASS for -mfpu=neon-fp-armv8

Comments

Kyrill Tkachov Feb. 23, 2016, 11:33 a.m. UTC | #1 
Hi Christian,

On 17/02/16 12:50, Christian Bruel wrote:
> target_option_current_node, used in c-pragma.c to check if a state
> should be popped or reseted to the previous value, was not set when
> switching state with #pragma GCC target (I missed to see that, since it
> is done for pop,reset). So in some cases the state might not be reset
> correctly.
>
> This patch sets it for #pragma GCC target paths and update the comments
> as well to clarify this point.
>
> As a benefit we now use this cached value instead of
> build_target_option_node (&global_options), this should speed up (a
> little bit) this path when processing arm_neon.h.
>
> One effect of it is that some predicate tests (e.g arm_neonv2_ok) in the
> testsuite was returning the wrong value, thus marking some test as
> UNRESOLVED instead of PASS. See the reduced case of the issue attached
> is the patch.
>
> Regtested, a few new PASS for -mfpu=neon-fp-armv8
>

Ok.
Thanks,
Kyrill
diff mbox

Patch

2016-02-17  Christian Bruel  <christian.bruel@st.com>

	* config/arm/arm-c.c (arm_option_override): Initialize
	target_option_current_node.
	* config/arm/arm.c (arm_pragma_target_parse): Replace
	build_target_option_node call by target_option_current_node.
	Set target_option_current_node.	Fix comments.
	
2016-02-17  Christian Bruel  <christian.bruel@st.com>

	* gcc.target/arm/pragma_cpp_fma.c: New test.

Index: gcc/config/arm/arm-c.c
===================================================================
--- gcc/config/arm/arm-c.c	(revision 233489)
+++ gcc/config/arm/arm-c.c	(working copy)
@@ -199,7 +199,7 @@  arm_cpu_cpp_builtins (struct cpp_reader * pfile)
 static bool
 arm_pragma_target_parse (tree args, tree pop_target)
 {
-  tree prev_tree = build_target_option_node (&global_options);
+  tree prev_tree = target_option_current_node;
   tree cur_tree;
   struct cl_target_option *prev_opt;
   struct cl_target_option *cur_opt;
@@ -220,9 +220,14 @@  arm_pragma_target_parse (tree args, tree pop_targe
 				    TREE_TARGET_OPTION (prev_tree));
 	  return false;
 	}
+
+      /* handle_pragma_pop_options and handle_pragma_reset_options will set
+       target_option_current_node, but not handle_pragma_target.  */
+      target_option_current_node = cur_tree;
     }
 
-  /* Figure out the previous mode.  */
+  /* Update macros if target_node changes. The global state will be restored
+     by arm_set_current_function.  */
   prev_opt  = TREE_TARGET_OPTION (prev_tree);
   cur_opt   = TREE_TARGET_OPTION (cur_tree);
 
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 233489)
+++ gcc/config/arm/arm.c	(working copy)
@@ -3446,7 +3446,8 @@  arm_option_override (void)
 
   /* Save the initial options in case the user does function specific
      options or #pragma target.  */
-  target_option_default_node = build_target_option_node (&global_options);
+  target_option_default_node = target_option_current_node
+      = build_target_option_node (&global_options);
 
   /* Init initial mode for testing.  */
   thumb_flipper = TARGET_THUMB;
Index: gcc/testsuite/gcc.target/arm/pragma_cpp_fma.c
===================================================================
--- gcc/testsuite/gcc.target/arm/pragma_cpp_fma.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/pragma_cpp_fma.c	(working copy)
@@ -0,0 +1,36 @@ 
+/* Test that FMA macro is correctly undefined.  */
+/* { dg-do compile } */
+/* { dg-skip-if "Default no fma" { *-*-* } { "-mfpu=*vfpv4*" "-mfpu=*armv8"} } */
+/* { dg-require-effective-target arm_fp_ok } */
+/* { dg-add-options arm_fp } */
+
+#pragma GCC push_options
+#pragma GCC target ("fpu=crypto-neon-fp-armv8")
+
+#ifndef __ARM_FEATURE_FMA
+#error "__ARM_FEATURE_FMA is not defined but should be"
+#endif
+
+#ifndef __ARM_FEATURE_CRYPTO
+#error "__ARM_FEATURE_CRYPTO is not defined but should be"
+#endif
+
+#if __ARM_NEON_FP != 6
+#error "__ARM_NEON_FP"
+#endif
+
+#if __ARM_FP != 14
+#error "__ARM_FP"
+#endif
+
+#pragma GCC pop_options
+
+#pragma GCC push_options
+#pragma GCC target ("fpu=neon-vfpv4")
+#pragma GCC pop_options
+
+#ifdef __ARM_FEATURE_FMA
+#error "__ARM_FEATURE_FMA is defined but should not be"
+#endif
+
+