Re: [PATCH, ARM] attribute target (thumb,arm) [2/6] respin (4th)

[Christian Bruel <christian dot bruel at st dot com>]

On 05/07/2015 10:49 AM, Ramana Radhakrishnan wrote:
> 
> 
> On 06/05/15 15:20, Christian Bruel wrote:
>> In preparation of the pragma target
>>
>> reorganize ÃTARGET_CPU_CPP_BUILTINSÃ to redefine mode dependent macros
>> based on current thumb_p.
> 
> I'm not entirely happy with this patch as it appears to be too tied to 
> just the "thumbness" of the attributes.

OK, I'll change arm_cpp_builtins (struct cpp_reader *in, bool thumb_p)
into arm_cpp_builtins (struct cpp_reader *in, int target_flags)

 Additionally there appears to be
> recomputing of booleans which could well have been done using the 
> standard headers, given that this includes tm.h why don't you have the 
> definitions from arm.h for the various TARGET_* macros. See a couple of 
> examples below.

This patch prepares the ground for attribute_target: the flags might not
be global, As I remember TARGET_THUMB checks
global_options.x_target_flags, but at that time of processing for the
#pragma target I think global_option is not adjusted.

see arm_pragma_target_parse: TARGET_THUMB != (TARGET_THUMB_P
(cur_opt->x_target_flags)


> 
> While you are here , can you look at moving all the CPP builtins into 
> one function and then working from there. That would be a better 
> restructuring in the long term rather than a piece meal move in this 
> case. Makes rebuild times smaller for folks by doing this in one place.

Sure,

> 
> 
> 
>>
>> Thanks,
>>
>> Christian
>>
> 
>> 2014-09-23  Christian Bruel  <christian.bruel@st.com>
>>
>> 	* config/arm/arm-c.c (cpp_def_or_undef): New functions.
>> 	(arm_cpp_builtins): Likewise.
>> 	* config/arm/arm.h (TARGET_CPU_CPP_BUILTINS): Move mode dependant
>> 	macros to arm_cpp_builtins.
>> 	* config/arm/arm-protos.h (arm_cpp_builtins): Declare.
>>
>> diff '--exclude=.svn' -ruN gnu_trunk.p1/gcc/gcc/config/arm/arm-c.c gnu_trunk.p2/gcc/gcc/config/arm/arm-c.c
>> --- gnu_trunk.p1/gcc/gcc/config/arm/arm-c.c	2015-05-06 14:06:27.508142998 +0200
>> +++ gnu_trunk.p2/gcc/gcc/config/arm/arm-c.c	2015-05-06 14:27:45.362310057 +0200
>> @@ -51,3 +51,73 @@
>>  {
>>    arm_lang_output_object_attributes_hook = arm_output_c_attributes;
>>  }
>> +
>> +/* Define or undefine macro.  */
>> +
>> +static void
>> +cpp_def_or_undef (struct cpp_reader *in, const char *str, bool def_p)
>> +{
>> +  if (def_p)
>> +    cpp_define (in, str);
>> +  else
>> +    cpp_undef (in, str);
>> +}
>> +
>> +/* Define or undefine macros based on the current target.  If the user does
>> +   #pragma GCC target, we need to adjust the macros dynamically.  */
>> +
>> +void
>> +arm_cpp_builtins (struct cpp_reader *in, bool thumb_p)
>> +{
>> +  bool target_32bit_p = !thumb_p || arm_arch_thumb2;
> 
> Why isn't this TARGET_32BIT ?

TARGET_32BIT checks global_options. We will need the options from the
current tree.

> 
>> +  bool thumb2_p = thumb_p && arm_arch_thumb2;
> 
> TARGET_THUMB2 ?

idem.

> 
>> +  bool have_ldrex_p = (arm_arch6 && !thumb_p) || arm_arch7;
>> +  bool have_ldrexbh_p = (arm_arch6k && !thumb_p) || arm_arch7;
>> +  bool have_ldrexd_p = ((arm_arch6k && !thumb_p) || arm_arch7)
>> +    && arm_arch_notm;
> 
> TARGET_HAVE_LDREX* ?

idem.

> 
>> +
>> +  int arm_feature_ldrex = (have_ldrex_p ? 4 : 0)
>> +    | (have_ldrexbh_p ? 3 : 0) | (have_ldrexd_p ? 8 : 0);
>> +
>> +  cpp_def_or_undef (in, "__thumb__", thumb_p);
>> +  if (arm_arch_thumb2)
>> +    cpp_def_or_undef (in, "__thumb2__", thumb_p);
>> +  if (TARGET_BIG_END)
>> +    cpp_def_or_undef (in, "__THUMBEB__", thumb_p);
>> +  else
>> +    cpp_def_or_undef (in, "__THUMBEL__", thumb_p);
>> +
>> +  cpp_def_or_undef (in, "__ARM_32BIT_STATE", target_32bit_p); /* TARGET_32BIT  */
>> +
>> +  if (arm_arch5e && (arm_arch_notm || arm_arch7))   /* TARGET_ARM_QBIT  */
>> +    cpp_def_or_undef (in, "__ARM_FEATURE_QBIT", target_32bit_p);
>> +
>> +  if (arm_arch6 && (arm_arch_notm || arm_arch7))    /* TARGET_ARM_SAT  */
>> +    cpp_def_or_undef (in, "__ARM_FEATURE_SAT", target_32bit_p);
>> +
>> +  if (arm_arch5e && (arm_arch_notm || arm_arch7em)) /* TARGET_DSP_MULTIPLY  */
>> +    cpp_def_or_undef (in, "__ARM_FEATURE_DSP", target_32bit_p);
>> +
>> +  if (arm_arch6 && (arm_arch_notm || arm_arch7em))  /* TARGET_INT_SIMD  */
>> +    cpp_def_or_undef (in, "__ARM_FEATURE_SIMD32", target_32bit_p);
>> +
>> + /* TARGET_IDIV  */
>> +  cpp_def_or_undef (in, "__ARM_ARCH_EXT_IDIV__",
>> +		    ((!thumb_p && arm_arch_arm_hwdiv)
>> +		     || (thumb2_p && arm_arch_thumb_hwdiv)));
>> +
>> +  cpp_def_or_undef (in, "__ARM_FEATURE_IDIV",
>> +		    ((!thumb_p && arm_arch_arm_hwdiv)
>> +		     || (thumb2_p && arm_arch_thumb_hwdiv)));
>> +
>> + if (arm_feature_ldrex)
>> +   cpp_define_formatted (in, "__ARM_FEATURE_LDREX=%d", arm_feature_ldrex);
>> + else
>> +   cpp_undef (in, "__ARM_FEATURE_LDREX");
>> +
>> + cpp_def_or_undef (in, "__ARM_FEATURE_CLZ",
>> +		   ((TARGET_ARM_ARCH >= 5 && !thumb_p) || TARGET_ARM_ARCH_ISA_THUMB >=2));
>> +
>> + cpp_def_or_undef (in, "__ARM_ASM_SYNTAX_UNIFIED__", inline_asm_unified);
>> +}
>> +
>> diff '--exclude=.svn' -ruN gnu_trunk.p1/gcc/gcc/config/arm/arm.h gnu_trunk.p2/gcc/gcc/config/arm/arm.h
>> --- gnu_trunk.p1/gcc/gcc/config/arm/arm.h	2015-05-06 14:24:41.149994939 +0200
>> +++ gnu_trunk.p2/gcc/gcc/config/arm/arm.h	2015-05-06 14:27:45.362310057 +0200
>> @@ -48,29 +48,12 @@
>>  #define TARGET_CPU_CPP_BUILTINS()			\
>>    do							\
>>      {							\
>> -	if (TARGET_DSP_MULTIPLY)			\
>> -	   builtin_define ("__ARM_FEATURE_DSP");	\
>> -        if (TARGET_ARM_QBIT)				\
>> -           builtin_define ("__ARM_FEATURE_QBIT");	\
>> -        if (TARGET_ARM_SAT)				\
>> -           builtin_define ("__ARM_FEATURE_SAT");	\
>>          if (TARGET_CRYPTO)				\
>>  	   builtin_define ("__ARM_FEATURE_CRYPTO");	\
>>  	if (unaligned_access)				\
>>  	  builtin_define ("__ARM_FEATURE_UNALIGNED");	\
>>  	if (TARGET_CRC32)				\
>>  	  builtin_define ("__ARM_FEATURE_CRC32");	\
>> -	if (TARGET_32BIT)				\
>> -	  builtin_define ("__ARM_32BIT_STATE");		\
>> -	if (TARGET_ARM_FEATURE_LDREX)				\
>> -	  builtin_define_with_int_value (			\
>> -	    "__ARM_FEATURE_LDREX", TARGET_ARM_FEATURE_LDREX);	\
>> -	if ((TARGET_ARM_ARCH >= 5 && !TARGET_THUMB)		\
>> -	     || TARGET_ARM_ARCH_ISA_THUMB >=2)			\
>> -	  builtin_define ("__ARM_FEATURE_CLZ");			\
>> -	if (TARGET_INT_SIMD)					\
>> -	  builtin_define ("__ARM_FEATURE_SIMD32");		\
>> -								\
>>  	builtin_define_with_int_value (				\
>>  	  "__ARM_SIZEOF_MINIMAL_ENUM",				\
>>  	  flag_short_enums ? 1 : 4);				\
>> @@ -89,10 +72,6 @@
>>  	if (arm_arch_notm)				\
>>  	  builtin_define ("__ARM_ARCH_ISA_ARM");	\
>>  	builtin_define ("__APCS_32__");			\
>> -	if (TARGET_THUMB)				\
>> -	  builtin_define ("__thumb__");			\
>> -	if (TARGET_THUMB2)				\
>> -	  builtin_define ("__thumb2__");		\
>>  	if (TARGET_ARM_ARCH_ISA_THUMB)			\
>>  	  builtin_define_with_int_value (		\
>>  	    "__ARM_ARCH_ISA_THUMB",			\
>> @@ -102,15 +81,9 @@
>>  	  {						\
>>  	    builtin_define ("__ARMEB__");		\
>>  	    builtin_define ("__ARM_BIG_ENDIAN");	\
>> -	    if (TARGET_THUMB)				\
>> -	      builtin_define ("__THUMBEB__");		\
>>  	  }						\
>>          else						\
>> -	  {						\
>>  	    builtin_define ("__ARMEL__");		\
>> -	    if (TARGET_THUMB)				\
>> -	      builtin_define ("__THUMBEL__");		\
>> -	  }						\
>>  							\
>>  	if (TARGET_SOFT_FLOAT)				\
>>  	  builtin_define ("__SOFTFP__");		\
>> @@ -163,13 +136,8 @@
>>  	      builtin_define ("__ARM_PCS");		\
>>  	    builtin_define ("__ARM_EABI__");		\
>>  	  }						\
>> -	if (TARGET_IDIV)				\
>> -         {						\
>> -            builtin_define ("__ARM_ARCH_EXT_IDIV__");	\
>> -            builtin_define ("__ARM_FEATURE_IDIV");	\
>> -         }						\
>> -	if (inline_asm_unified)				\
>> -	  builtin_define ("__ARM_ASM_SYNTAX_UNIFIED__");\
>> +	/* Remaining macros depends on TARGET_THUMB.  */\
>> +        arm_cpp_builtins (pfile, TARGET_THUMB);		\
>>      } while (0)
> 
>>
>>  #include "config/arm/arm-opts.h"
>> diff '--exclude=.svn' -ruN gnu_trunk.p1/gcc/gcc/config/arm/arm-protos.h gnu_trunk.p2/gcc/gcc/config/arm/arm-protos.h
>> --- gnu_trunk.p1/gcc/gcc/config/arm/arm-protos.h	2015-05-06 14:06:26.324141030 +0200
>> +++ gnu_trunk.p2/gcc/gcc/config/arm/arm-protos.h	2015-05-06 14:27:45.362310057 +0200
>> @@ -218,8 +218,6 @@
>>  extern void arm_pr_no_long_calls (struct cpp_reader *);
>>  extern void arm_pr_long_calls_off (struct cpp_reader *);
>>
>> -extern void arm_lang_object_attributes_init(void);
>> -
>>  extern const char *arm_mangle_type (const_tree);
>>  extern const char *arm_mangle_builtin_type (const_tree);
>>
>> @@ -324,6 +322,10 @@
>>  /* Defined in gcc/common/config/arm-common.c.  */
>>  extern const char *arm_rewrite_selected_cpu (const char *name);
>>
>> +/* Defined in gcc/common/config/arm-c.c.  */
>> +extern void arm_lang_object_attributes_init(void);
>> +extern void arm_cpp_builtins (struct cpp_reader *, bool);
>> +
>>  extern bool arm_is_constant_pool_ref (rtx);
>>
>>  /* Flags used to identify the presence of processor capabilities.  */
>