Re: [PATCH ARM]: PR67745: Fix function alignment after __attribute__ 2/2

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

> 
> yes I see, I was hoping to avoid a new hook, but as you said it seems mandatory for the mere declaration case.
> 
> Here is one proposal, it defaults to nothing and the ARM implementation does not need to handle the vptr bit setting. so that simplifies a lot the things.
> 
> The hook is called from rest_of_decl_compilation for mere declarations and allocate_struct_function for definitions.


I'm not sure we have testsuite coverage for this -  can you add a test or 2 ?

> 
> 
> 
> 
> 
> 
> 
> align_hook.patch
> 
> 
> 2015-09-29  Christian Bruel  <christian.bruel@st.com>
> 
> 	PR target/67745
> 	* config/arm/arm.h (FUNCTION_BOUNDARY): Use FUNCTION_BOUNDARY_P.
> 	(FUNCTION_BOUNDARY_P): New macro:
> 	* config/arm/arm.c (TARGET_RELAYOUT_FUNCTION, arm_relayout_function): 
> 	New hook.

The ARM changes look ok to me. Please as usual watch out for any regressions.



> 	* doc/tm.texi.in (TARGET_RELAYOUT_FUNCTION): Document.
> 	* doc/tm.texi (TARGET_RELAYOUT_FUNCTION): New hook.
> 	* gcc/target.def (TARGET_RELAYOUT_FUNCTION): Likewise.
> 	* gcc/function.c (allocate_struct_function): Call relayout_function hook.
> 	* gcc/passes.c (rest_of_decl_compilation): Likewise.
> 	* gcc/targhooks.c (default_relayout_function): New function.
> 	* gcc/targhooks.h (default_relayout_function): Declare.
> 


> --- gnu_trunk.p0/gcc/gcc/config/arm/arm.c	2015-10-12 10:34:27.599740376 +0200
> +++ gnu_trunk.devs/gcc/gcc/config/arm/arm.c	2015-10-12 12:26:51.607398021 +0200
> @@ -250,6 +250,7 @@ static void arm_override_options_after_c
>  static void arm_option_print (FILE *, int, struct cl_target_option *);
>  static void arm_set_current_function (tree);
>  static bool arm_can_inline_p (tree, tree);
> +static void arm_relayout_function (tree);
>  static bool arm_valid_target_attribute_p (tree, tree, tree, int);
>  static unsigned HOST_WIDE_INT arm_shift_truncation_mask (machine_mode);
>  static bool arm_macro_fusion_p (void);
> @@ -405,6 +406,9 @@ static const struct attribute_spec arm_a
>  #undef TARGET_CAN_INLINE_P
>  #define TARGET_CAN_INLINE_P arm_can_inline_p
>  
> +#undef TARGET_RELAYOUT_FUNCTION
> +#define TARGET_RELAYOUT_FUNCTION arm_relayout_function
> +
>  #undef  TARGET_OPTION_OVERRIDE
>  #define TARGET_OPTION_OVERRIDE arm_option_override
>  
> @@ -29825,6 +29829,23 @@ arm_can_inline_p (tree caller ATTRIBUTE_
>    return true;
>  }
>  
> +/* Hook to fix function's alignment affected by target attribute.  */
> +
> +static void
> +arm_relayout_function (tree fndecl)
> +{
> +  if (DECL_USER_ALIGN (fndecl))
> +    return;
> +
> +  tree callee_tree = DECL_FUNCTION_SPECIFIC_TARGET (fndecl);
> +
> +  if (!callee_tree)
> +    callee_tree = target_option_default_node;
> +
> +  DECL_ALIGN (fndecl) =
> +    FUNCTION_BOUNDARY_P (TREE_TARGET_OPTION (callee_tree)->x_target_flags);
> +}
> +
>  /* Inner function to process the attribute((target(...))), take an argument and
>     set the current options from the argument.  If we have a list, recursively
>     go over the list.  */
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/config/arm/arm.h gnu_trunk.devs/gcc/gcc/config/arm/arm.h
> --- gnu_trunk.p0/gcc/gcc/config/arm/arm.h	2015-10-12 10:34:27.607740391 +0200
> +++ gnu_trunk.devs/gcc/gcc/config/arm/arm.h	2015-10-12 12:27:55.507546958 +0200
> @@ -565,7 +565,8 @@ extern int arm_arch_crc;
>  #define PREFERRED_STACK_BOUNDARY \
>      (arm_abi == ARM_ABI_ATPCS ? 64 : STACK_BOUNDARY)
>  
> -#define FUNCTION_BOUNDARY           (TARGET_THUMB ? 16 : 32)
> +#define FUNCTION_BOUNDARY_P(flags)  (TARGET_THUMB_P (flags) ? 16 : 32)
> +#define FUNCTION_BOUNDARY           (FUNCTION_BOUNDARY_P (target_flags))
>  
>  /* The lowest bit is used to indicate Thumb-mode functions, so the
>     vbit must go into the delta field of pointers to member
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/doc/tm.texi gnu_trunk.devs/gcc/gcc/doc/tm.texi
> --- gnu_trunk.p0/gcc/gcc/doc/tm.texi	2015-10-12 10:33:29.907630642 +0200
> +++ gnu_trunk.devs/gcc/gcc/doc/tm.texi	2015-10-12 12:33:33.880332253 +0200
> @@ -9985,6 +9985,10 @@ default, inlining is not allowed if the
>  specific target options and the caller does not use the same options.
>  @end deftypefn
>  
> +@deftypefn {Target Hook} void TARGET_RELAYOUT_FUNCTION (tree @var{fndecl})
> +This target hook fixes function @var{fndecl} after attributes are processed. Default does nothing. On ARM, the default function's alignment is updated with the attribute target.
> +@end deftypefn
> +
>  @node Emulated TLS
>  @section Emulating TLS
>  @cindex Emulated TLS
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/doc/tm.texi.in gnu_trunk.devs/gcc/gcc/doc/tm.texi.in
> --- gnu_trunk.p0/gcc/gcc/doc/tm.texi.in	2015-10-12 10:33:29.919630666 +0200
> +++ gnu_trunk.devs/gcc/gcc/doc/tm.texi.in	2015-10-12 11:28:16.350590629 +0200
> @@ -7274,6 +7274,8 @@ on this implementation detail.
>  
>  @hook TARGET_CAN_INLINE_P
>  
> +@hook TARGET_RELAYOUT_FUNCTION
> +
>  @node Emulated TLS
>  @section Emulating TLS
>  @cindex Emulated TLS
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/function.c gnu_trunk.devs/gcc/gcc/function.c
> --- gnu_trunk.p0/gcc/gcc/function.c	2015-10-12 10:33:32.715635978 +0200
> +++ gnu_trunk.devs/gcc/gcc/function.c	2015-10-12 10:04:42.764482359 +0200
> @@ -4840,6 +4840,9 @@ allocate_struct_function (tree fndecl, b
>  	  for (tree parm = DECL_ARGUMENTS (fndecl); parm;
>  	       parm = DECL_CHAIN (parm))
>  	    relayout_decl (parm);
> +
> +	  /* Similarly relayout the function decl.  */
> +	  targetm.target_option.relayout_function (fndecl);
>  	}
>  
>        if (!abstract_p && aggregate_value_p (result, fndecl))
> --- gnu_trunk.p0/gcc/gcc/passes.c	2015-10-05 14:22:31.082400039 +0200
> +++ gnu_trunk.devs/gcc/gcc/passes.c	2015-10-12 11:31:55.343150983 +0200
> @@ -253,6 +253,11 @@ rest_of_decl_compilation (tree decl,
>  	}
>  #endif
>  
> +      /* Now that we have activated any function-specific attributes
> +	 that might affect function decl, particularly align, relayout it.  */
> +      if (TREE_CODE (decl) == FUNCTION_DECL)
> +	targetm.target_option.relayout_function (decl);
> +
>        timevar_pop (TV_VARCONST);
>      }
>    else if (TREE_CODE (decl) == TYPE_DECL
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/target.def gnu_trunk.devs/gcc/gcc/target.def
> --- gnu_trunk.p0/gcc/gcc/target.def	2015-10-05 14:22:20.142374948 +0200
> +++ gnu_trunk.devs/gcc/gcc/target.def	2015-10-12 12:34:23.744447530 +0200
> @@ -5620,6 +5620,12 @@ specific target options and the caller d
>   bool, (tree caller, tree callee),
>   default_target_can_inline_p)
>  
> +DEFHOOK
> +(relayout_function,
> +"This target hook fixes function @var{fndecl} after attributes are processed. Default does nothing. On ARM, the default function's alignment is updated with the attribute target.",
> + void, (tree fndecl),
> + default_relayout_function)

Can this just be hook_void_tree ? Then you don't need the empty default_relayout_function ?




> +
>  HOOK_VECTOR_END (target_option)
>  
>  /* For targets that need to mark extra registers as live on entry to
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/targhooks.c gnu_trunk.devs/gcc/gcc/targhooks.c
> --- gnu_trunk.p0/gcc/gcc/targhooks.c	2015-08-25 10:28:00.504946542 +0200
> +++ gnu_trunk.devs/gcc/gcc/targhooks.c	2015-10-12 11:24:50.866063124 +0200
> @@ -1311,6 +1311,11 @@ default_target_option_pragma_parse (tree
>    return false;
>  }
>  
> +void
> +default_relayout_function (tree fndecl)
> +{
> +}
> +
>  bool
>  default_target_can_inline_p (tree caller, tree callee)
>  {
> diff '--exclude=.svn' '--exclude=*~' -r -up gnu_trunk.p0/gcc/gcc/targhooks.h gnu_trunk.devs/gcc/gcc/targhooks.h
> --- gnu_trunk.p0/gcc/gcc/targhooks.h	2015-10-05 14:22:17.774369515 +0200
> +++ gnu_trunk.devs/gcc/gcc/targhooks.h	2015-10-12 11:24:40.914042025 +0200
> @@ -161,6 +161,7 @@ extern bool default_hard_regno_scratch_o
>  extern bool default_mode_dependent_address_p (const_rtx, addr_space_t);
>  extern bool default_target_option_valid_attribute_p (tree, tree, tree, int);
>  extern bool default_target_option_pragma_parse (tree, tree);
> +extern void default_relayout_function (tree);
>  extern bool default_target_can_inline_p (tree, tree);
>  extern bool default_valid_pointer_mode (machine_mode);
>  extern bool default_ref_may_alias_errno (struct ao_ref *);
>