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

[Ramana Radhakrishnan <ramana.radhakrishnan@arm.com>]

> 2014-09-23  Christian Bruel  <christian.bruel@st.com>
>
> 	* config/arm/arm.h (arm_option_override): Reoganized and split.

Reorganized and split into ....


> 	(arm_option_params_internal); New function.

s/;/: ".... New function."
> 	(arm_option_check_internal): New function.
> 	(arm_option_override_internal): New function.
> 	(restrict_default): New boolean.

Non existent ?


> 	(thumb_code, thumb1_code): Remove.
> 	* config/arm/arm.h (TREE_TARGET_THUMB, TREE_TARGET_THUMB1): New macros.
> 	(TREE_TARGET_THUM2, TREE_TARGET_ARM): Likewise.
> 	(thumb_code, thumb1_code): Remove.
> 	* config/arm/arm.md (is_thumb, is_thumb1): Check TARGET flag.
>
> diff '--exclude=.svn' -ruN gnu_trunk.ref/gcc/gcc/config/arm/arm.c gnu_trunk.p1/gcc/gcc/config/arm/arm.c
> --- gnu_trunk.ref/gcc/gcc/config/arm/arm.c	2015-05-05 14:35:30.214153999 +0200
> +++ gnu_trunk.p1/gcc/gcc/config/arm/arm.c	2015-05-06 14:24:41.125994898 +0200
> @@ -846,12 +846,6 @@
>  /* Nonzero if tuning for Cortex-A9.  */
>  int arm_tune_cortex_a9 = 0;
>
> -/* Nonzero if generating Thumb instructions.  */
> -int thumb_code = 0;
> -
> -/* Nonzero if generating Thumb-1 instructions.  */
> -int thumb1_code = 0;
> -
>  /* Nonzero if we should define __THUMB_INTERWORK__ in the
>     preprocessor.
>     XXX This is a bit of a hack, it's intended to help work around
> @@ -2669,6 +2663,148 @@
>    return std_gimplify_va_arg_expr (valist, type, pre_p, post_p);
>  }
>
> +/* Check any incompatible options that the user has specified.  */
> +static void
> +arm_option_check_internal (struct gcc_options *opts)
> +{
> +  /* Make sure that the processor choice does not conflict with any of the
> +     other command line choices.  */
> +  if (TREE_TARGET_ARM (opts) && !(insn_flags & FL_NOTM))
> +    error ("target CPU does not support ARM mode");
> +
> +  /* TARGET_BACKTRACE calls leaf_function_p, which causes a crash if done
> +     from here where no function is being compiled currently.  */
> +  if ((TARGET_TPCS_FRAME || TARGET_TPCS_LEAF_FRAME) && TREE_TARGET_ARM (opts))
> +    warning (0, "enabling backtrace support is only meaningful when compiling for the Thumb");
> +
> +  if (TREE_TARGET_ARM (opts) && TARGET_CALLEE_INTERWORKING)
> +    warning (0, "enabling callee interworking support is only meaningful when compiling for the Thumb");
> +
> +  /* If this target is normally configured to use APCS frames, warn if they
> +     are turned off and debugging is turned on.  */
> +  if (TREE_TARGET_ARM (opts)
> +      && write_symbols != NO_DEBUG
> +      && !TARGET_APCS_FRAME
> +      && (TARGET_DEFAULT & MASK_APCS_FRAME))
> +    warning (0, "-g with -mno-apcs-frame may not give sensible debugging");
> +
> +  /* iWMMXt unsupported under Thumb mode.  */
> +  if (TREE_TARGET_THUMB (opts) && TARGET_IWMMXT)
> +    error ("iWMMXt unsupported under Thumb mode");
> +
> +  if (TARGET_HARD_TP && TREE_TARGET_THUMB1 (opts))
> +    error ("can not use -mtp=cp15 with 16-bit Thumb");
> +
> +  if (TREE_TARGET_THUMB (opts) && TARGET_VXWORKS_RTP && flag_pic)
> +    {
> +      error ("RTP PIC is incompatible with Thumb");
> +      flag_pic = 0;
> +    }
> +
> +  /* We only support -mslow-flash-data on armv7-m targets.  */
> +  if (target_slow_flash_data
> +      && ((!(arm_arch7 && !arm_arch_notm) && !arm_arch7em)
> +	  || (TREE_TARGET_THUMB1 (opts) || flag_pic || TARGET_NEON)))
> +    error ("-mslow-flash-data only supports non-pic code on armv7-m targets");
> +}
> +
> +/* Check any params depending on attributes that the user has specified.  */

Set params depending on target and optimization options.

> +static void
> +arm_option_params_internal (struct gcc_options *opts)
> +{
> + /* If we are not using the default (ARM mode) section anchor offset
> +     ranges, then set the correct ranges now.  */
> +  if (TREE_TARGET_THUMB1 (opts))
> +    {
> +      /* Thumb-1 LDR instructions cannot have negative offsets.
> +         Permissible positive offset ranges are 5-bit (for byte loads),
> +         6-bit (for halfword loads), or 7-bit (for word loads).
> +         Empirical results suggest a 7-bit anchor range gives the best
> +         overall code size.  */
> +      targetm.min_anchor_offset = 0;
> +      targetm.max_anchor_offset = 127;
> +    }
> +  else if (TREE_TARGET_THUMB2 (opts))
> +    {
> +      /* The minimum is set such that the total size of the block
> +         for a particular anchor is 248 + 1 + 4095 bytes, which is
> +         divisible by eight, ensuring natural spacing of anchors.  */
> +      targetm.min_anchor_offset = -248;
> +      targetm.max_anchor_offset = 4095;
> +    }
> +  else
> +    {
> +      targetm.min_anchor_offset = TARGET_MIN_ANCHOR_OFFSET;
> +      targetm.max_anchor_offset = TARGET_MAX_ANCHOR_OFFSET;
> +    }
> +
> +  if (optimize_size)
> +    {
> +      /* If optimizing for size, bump the number of instructions that we
> +         are prepared to conditionally execute (even on a StrongARM).  */
> +      max_insns_skipped = 6;
> +
> +      /* For THUMB2, we limit the conditional sequence to one IT block.  */
> +      if (TREE_TARGET_THUMB2 (opts))
> +	max_insns_skipped = opts->x_arm_restrict_it ? 1 : 4;

Not sure about indentation here,

> +    }


Let's move the else part also here from arm_option_override - I'd rather 
keep these together than have to worry about this separately later.

 > +  if (! optimize_size)
 > +    max_insns_skipped = current_tune->max_insns_skipped;
 > +


> +}
> +
> +/* Reset options between modes that the user has specified.  */
> +static void
> +arm_option_override_internal (struct gcc_options *opts,
> +			      struct gcc_options *opts_set)
> +{
> +  if (TREE_TARGET_THUMB (opts) && !(insn_flags & FL_THUMB))
> +    {
> +      warning (0, "target CPU does not support THUMB instructions");
> +      opts->x_target_flags &= ~MASK_THUMB;
> +    }
> +
> +  if (TARGET_APCS_FRAME && TREE_TARGET_THUMB (opts))
> +    {
> +      /* warning (0, "ignoring -mapcs-frame because -mthumb was used"); */
> +      opts->x_target_flags &= ~MASK_APCS_FRAME;
> +    }
> +
> +  /* Callee super interworking implies thumb interworking.  Adding
> +     this to the flags here simplifies the logic elsewhere.  */
> +  if (TREE_TARGET_THUMB (opts) && TARGET_CALLEE_INTERWORKING)
> +    opts->x_target_flags |= MASK_INTERWORK;
> +
> +  if (! opts_set->x_arm_restrict_it)
> +    opts->x_arm_restrict_it = arm_arch8;
> +
> +  if (!TREE_TARGET_THUMB2 (opts))
> +    opts->x_arm_restrict_it = 0;
> +
> +  if (TREE_TARGET_THUMB1 (opts))
> +    {
> +      /* Don't warn since it's on by default in -O2.  */
> +      opts->x_flag_schedule_insns = 0;
> +    }
> +
> +  /* Disable shrink-wrap when optimizing function for size, since it tends to
> +     generate additional returns.  */
> +  if (optimize_function_for_size_p (cfun) && TREE_TARGET_THUMB2 (opts))
> +    opts->x_flag_shrink_wrap = false;
> +
> +  /* In Thumb1 mode, we emit the epilogue in RTL, but the last insn
> +     - epilogue_insns - does not accurately model the corresponding insns
> +     emitted in the asm file.  In particular, see the comment in thumb_exit
> +     'Find out how many of the (return) argument registers we can corrupt'.
> +     As a consequence, the epilogue may clobber registers without fipa-ra
> +     finding out about it.  Therefore, disable fipa-ra in Thumb1 mode.
> +     TODO: Accurately model clobbers for epilogue_insns and reenable
> +     fipa-ra.  */
> +  if (TREE_TARGET_THUMB1 (opts))
> +    opts->x_flag_ipa_ra = 0;
> +
> +  /* Thumb2 inline assembly code should always use unified syntax.
> +     This will apply to ARM and Thumb1 eventually.  */
> +  opts->x_inline_asm_unified = TREE_TARGET_THUMB2 (opts);
> +}
> +
>  /* Fix up any incompatible options that the user has specified.  */
>  static void
>  arm_option_override (void)
> @@ -2815,10 +2951,9 @@
>    tune_flags = arm_selected_tune->flags;
>    current_tune = arm_selected_tune->tune;
>
> -  /* Make sure that the processor choice does not conflict with any of the
> -     other command line choices.  */
> -  if (TARGET_ARM && !(insn_flags & FL_NOTM))
> -    error ("target CPU does not support ARM mode");
> +  /* TBD: Dwarf info for apcs frame is not handled yet.  */
> +  if (TARGET_APCS_FRAME)
> +    flag_shrink_wrap = false;
>
>    /* BPABI targets use linker tricks to allow interworking on cores
>       without thumb support.  */
> @@ -2828,31 +2963,6 @@
>        target_flags &= ~MASK_INTERWORK;
>      }
>
> -  if (TARGET_THUMB && !(insn_flags & FL_THUMB))
> -    {
> -      warning (0, "target CPU does not support THUMB instructions");
> -      target_flags &= ~MASK_THUMB;
> -    }
> -
> -  if (TARGET_APCS_FRAME && TARGET_THUMB)
> -    {
> -      /* warning (0, "ignoring -mapcs-frame because -mthumb was used"); */
> -      target_flags &= ~MASK_APCS_FRAME;
> -    }
> -
> -  /* Callee super interworking implies thumb interworking.  Adding
> -     this to the flags here simplifies the logic elsewhere.  */
> -  if (TARGET_THUMB && TARGET_CALLEE_INTERWORKING)
> -    target_flags |= MASK_INTERWORK;
> -
> -  /* TARGET_BACKTRACE calls leaf_function_p, which causes a crash if done
> -     from here where no function is being compiled currently.  */
> -  if ((TARGET_TPCS_FRAME || TARGET_TPCS_LEAF_FRAME) && TARGET_ARM)
> -    warning (0, "enabling backtrace support is only meaningful when compiling for the Thumb");
> -
> -  if (TARGET_ARM && TARGET_CALLEE_INTERWORKING)
> -    warning (0, "enabling callee interworking support is only meaningful when compiling for the Thumb");
> -
>    if (TARGET_APCS_STACK && !TARGET_APCS_FRAME)
>      {
>        warning (0, "-mapcs-stack-check incompatible with -mno-apcs-frame");
> @@ -2868,14 +2978,6 @@
>    if (TARGET_APCS_REENT)
>      warning (0, "APCS reentrant code not supported.  Ignored");
>
> -  /* If this target is normally configured to use APCS frames, warn if they
> -     are turned off and debugging is turned on.  */
> -  if (TARGET_ARM
> -      && write_symbols != NO_DEBUG
> -      && !TARGET_APCS_FRAME
> -      && (TARGET_DEFAULT & MASK_APCS_FRAME))
> -    warning (0, "-g with -mno-apcs-frame may not give sensible debugging");
> -
>    if (TARGET_APCS_FLOAT)
>      warning (0, "passing floating point arguments in fp regs not yet supported");
>
> @@ -2897,8 +2999,6 @@
>
>    arm_ld_sched = (tune_flags & FL_LDSCHED) != 0;
>    arm_tune_strongarm = (tune_flags & FL_STRONG) != 0;
> -  thumb_code = TARGET_ARM == 0;
> -  thumb1_code = TARGET_THUMB1 != 0;
>    arm_tune_wbuf = (tune_flags & FL_WBUF) != 0;
>    arm_tune_xscale = (tune_flags & FL_XSCALE) != 0;
>    arm_arch_iwmmxt = (insn_flags & FL_IWMMXT) != 0;
> @@ -2909,32 +3009,6 @@
>    arm_tune_cortex_a9 = (arm_tune == cortexa9) != 0;
>    arm_arch_crc = (insn_flags & FL_CRC32) != 0;
>    arm_m_profile_small_mul = (insn_flags & FL_SMALLMUL) != 0;
> -  if (arm_restrict_it == 2)
> -    arm_restrict_it = arm_arch8 && TARGET_THUMB2;
> -
> -  if (!TARGET_THUMB2)
> -    arm_restrict_it = 0;
> -
> -  /* If we are not using the default (ARM mode) section anchor offset
> -     ranges, then set the correct ranges now.  */
> -  if (TARGET_THUMB1)
> -    {
> -      /* Thumb-1 LDR instructions cannot have negative offsets.
> -         Permissible positive offset ranges are 5-bit (for byte loads),
> -         6-bit (for halfword loads), or 7-bit (for word loads).
> -         Empirical results suggest a 7-bit anchor range gives the best
> -         overall code size.  */
> -      targetm.min_anchor_offset = 0;
> -      targetm.max_anchor_offset = 127;
> -    }
> -  else if (TARGET_THUMB2)
> -    {
> -      /* The minimum is set such that the total size of the block
> -         for a particular anchor is 248 + 1 + 4095 bytes, which is
> -         divisible by eight, ensuring natural spacing of anchors.  */
> -      targetm.min_anchor_offset = -248;
> -      targetm.max_anchor_offset = 4095;
> -    }
>
>    /* V5 code we generate is completely interworking capable, so we turn off
>       TARGET_INTERWORK here to avoid many tests later on.  */
> @@ -2953,6 +3027,9 @@
>    if (TARGET_IWMMXT_ABI && !TARGET_IWMMXT)
>      error ("iwmmxt abi requires an iwmmxt capable cpu");
>
> +  if (! optimize_size)
> +    max_insns_skipped = current_tune->max_insns_skipped;
> +

Please move this as an else part to the same in arm_options_params_internal.

>    if (!global_options_set.x_arm_fpu_index)
>      {
>        const char *target_fpu_name;
> @@ -2994,10 +3071,6 @@
>    if (TARGET_IWMMXT && TARGET_NEON)
>      error ("iWMMXt and NEON are incompatible");
>
> -  /* iWMMXt unsupported under Thumb mode.  */
> -  if (TARGET_THUMB && TARGET_IWMMXT)
> -    error ("iWMMXt unsupported under Thumb mode");
> -
>    /* __fp16 support currently assumes the core has ldrh.  */
>    if (!arm_arch4 && arm_fp16_format != ARM_FP16_FORMAT_NONE)
>      sorry ("__fp16 and no ldrh");
> @@ -3042,9 +3115,6 @@
>  	target_thread_pointer = TP_SOFT;
>      }
>
> -  if (TARGET_HARD_TP && TARGET_THUMB1)
> -    error ("can not use -mtp=cp15 with 16-bit Thumb");
> -
>    /* Override the default structure alignment for AAPCS ABI.  */
>    if (!global_options_set.x_arm_structure_size_boundary)
>      {
> @@ -3067,12 +3137,6 @@
>  	}
>      }
>
> -  if (!TARGET_ARM && TARGET_VXWORKS_RTP && flag_pic)
> -    {
> -      error ("RTP PIC is incompatible with Thumb");
> -      flag_pic = 0;
> -    }
> -
>    /* If stack checking is disabled, we can use r10 as the PIC register,
>       which keeps r9 available.  The EABI specifies r9 as the PIC register.  */
>    if (flag_pic && TARGET_SINGLE_PIC_BASE)
> @@ -3140,25 +3204,6 @@
>        unaligned_access = 0;
>      }
>
> -  if (TARGET_THUMB1 && flag_schedule_insns)
> -    {
> -      /* Don't warn since it's on by default in -O2.  */
> -      flag_schedule_insns = 0;
> -    }
> -
> -  if (optimize_size)
> -    {
> -      /* If optimizing for size, bump the number of instructions that we
> -         are prepared to conditionally execute (even on a StrongARM).  */
> -      max_insns_skipped = 6;
> -
> -      /* For THUMB2, we limit the conditional sequence to one IT block.  */
> -      if (TARGET_THUMB2)
> -	max_insns_skipped = MAX_INSN_PER_IT_BLOCK;
> -    }
> -  else
> -    max_insns_skipped = current_tune->max_insns_skipped;
> -
>    /* Hot/Cold partitioning is not currently supported, since we can't
>       handle literal pool placement in that case.  */
>    if (flag_reorder_blocks_and_partition)
> @@ -3236,45 +3281,19 @@
>                           global_options.x_param_values,
>                           global_options_set.x_param_values);
>
> -  /* Disable shrink-wrap when optimizing function for size, since it tends to
> -     generate additional returns.  */
> -  if (optimize_function_for_size_p (cfun) && TARGET_THUMB2)
> -    flag_shrink_wrap = false;
> -  /* TBD: Dwarf info for apcs frame is not handled yet.  */
> -  if (TARGET_APCS_FRAME)
> -    flag_shrink_wrap = false;
> -
> -  /* We only support -mslow-flash-data on armv7-m targets.  */
> -  if (target_slow_flash_data
> -      && ((!(arm_arch7 && !arm_arch_notm) && !arm_arch7em)
> -	  || (TARGET_THUMB1 || flag_pic || TARGET_NEON)))
> -    error ("-mslow-flash-data only supports non-pic code on armv7-m targets");
> -
>    /* Currently, for slow flash data, we just disable literal pools.  */
>    if (target_slow_flash_data)
>      arm_disable_literal_pool = true;
>
> -  /* Thumb2 inline assembly code should always use unified syntax.
> -     This will apply to ARM and Thumb1 eventually.  */
> -  if (TARGET_THUMB2)
> -    inline_asm_unified = 1;
> -
>    /* Disable scheduling fusion by default if it's not armv7 processor
>       or doesn't prefer ldrd/strd.  */
>    if (flag_schedule_fusion == 2
>        && (!arm_arch7 || !current_tune->prefer_ldrd_strd))
>      flag_schedule_fusion = 0;
>
> -  /* In Thumb1 mode, we emit the epilogue in RTL, but the last insn
> -     - epilogue_insns - does not accurately model the corresponding insns
> -     emitted in the asm file.  In particular, see the comment in thumb_exit
> -     'Find out how many of the (return) argument registers we can corrupt'.
> -     As a consequence, the epilogue may clobber registers without fipa-ra
> -     finding out about it.  Therefore, disable fipa-ra in Thumb1 mode.
> -     TODO: Accurately model clobbers for epilogue_insns and reenable
> -     fipa-ra.  */
> -  if (TARGET_THUMB1)
> -    flag_ipa_ra = 0;
> +  arm_option_override_internal (&global_options, &global_options_set);
> +  arm_option_check_internal (&global_options);
> +  arm_option_params_internal (&global_options);
>
>    /* Register global variables with the garbage collector.  */
>    arm_add_gc_roots ();
> diff '--exclude=.svn' -ruN gnu_trunk.ref/gcc/gcc/config/arm/arm.h gnu_trunk.p1/gcc/gcc/config/arm/arm.h
> --- gnu_trunk.ref/gcc/gcc/config/arm/arm.h	2015-04-27 09:46:13.485992934 +0200
> +++ gnu_trunk.p1/gcc/gcc/config/arm/arm.h	2015-05-06 14:24:41.149994939 +0200
> @@ -252,6 +252,13 @@
>  #define SUBTARGET_CPP_SPEC      ""
>  #endif
>  \f
> +/* Tree Target Specification.  */
> +#define TREE_TARGET_THUMB(opts)  (TARGET_THUMB_P (opts->x_target_flags))
> +#define TREE_TARGET_ARM(opts)    (!TARGET_THUMB_P (opts->x_target_flags))
> +#define TREE_TARGET_THUMB1(opts) (TARGET_THUMB_P (opts->x_target_flags) \
> +				  && !arm_arch_thumb2)
> +#define TREE_TARGET_THUMB2(opts) (TARGET_THUMB_P (opts->x_target_flags) \
> +				  && arm_arch_thumb2)
>  /* Run-time Target Specification.  */
>  #define TARGET_SOFT_FLOAT		(arm_float_abi == ARM_FLOAT_ABI_SOFT)
>  /* Use hardware floating point instructions. */
> diff '--exclude=.svn' -ruN gnu_trunk.ref/gcc/gcc/config/arm/arm.md gnu_trunk.p1/gcc/gcc/config/arm/arm.md
> --- gnu_trunk.ref/gcc/gcc/config/arm/arm.md	2015-04-29 08:53:25.093082252 +0200
> +++ gnu_trunk.p1/gcc/gcc/config/arm/arm.md	2015-05-06 14:24:41.161994961 +0200
> @@ -69,13 +69,17 @@
>  ; IS_THUMB is set to 'yes' when we are generating Thumb code, and 'no' when
>  ; generating ARM code.  This is used to control the length of some insn
>  ; patterns that share the same RTL in both ARM and Thumb code.
> -(define_attr "is_thumb" "no,yes" (const (symbol_ref "thumb_code")))
> +(define_attr "is_thumb" "yes,no"
> +  (const (if_then_else (symbol_ref "TARGET_THUMB")
> +		       (const_string "yes") (const_string "no"))))
>
>  ; IS_ARCH6 is set to 'yes' when we are generating code form ARMv6.
>  (define_attr "is_arch6" "no,yes" (const (symbol_ref "arm_arch6")))
>
>  ; IS_THUMB1 is set to 'yes' iff we are generating Thumb-1 code.
> -(define_attr "is_thumb1" "no,yes" (const (symbol_ref "thumb1_code")))
> +(define_attr "is_thumb1" "yes,no"
> +  (const (if_then_else (symbol_ref "TARGET_THUMB1")
> +		       (const_string "yes") (const_string "no"))))
>
>  ; We use this attribute to disable alternatives that can produce 32-bit
>  ; instructions inside an IT-block in Thumb2 state.  ARMv8 deprecates IT blocks


OK with those changes.



Ramana