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.br...@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.  */
> 

Reply via email to