This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/7] [ARC] Add support for naked functions.


* Claudiu Zissulescu <Claudiu.Zissulescu@synopsys.com> [2017-06-01 15:34:51 +0200]:

> gcc/
> 2016-12-13  Claudiu Zissulescu  <claziss@synopsys.com>
> 	    Andrew Burgess  <andrew.burgess@embecosm.com>
> 
> 	* config/arc/arc-protos.h (arc_compute_function_type): Change prototype.
> 	(arc_return_address_register): New function.
> 	* config/arc/arc.c (arc_handle_fndecl_attribute): New function.
> 	(arc_handle_fndecl_attribute): Add naked attribute.
> 	(TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS): Define.
> 	(TARGET_WARN_FUNC_RETURN): Likewise.
> 	(arc_allocate_stack_slots_for_args): New function.
> 	(arc_warn_func_return): Likewise.
> 	(machine_function): Change type fn_type.
> 	(arc_compute_function_type): Consider new naked function type,
> 	change function return type.
> 	(arc_must_save_register): Adapt to handle new
> 	arc_compute_function_type's return type.
> 	(arc_expand_prologue): Likewise.
> 	(arc_expand_epilogue): Likewise.
> 	(arc_return_address_regs): Delete.
> 	(arc_return_address_register): New function.
> 	(arc_epilogue_uses): Use above function.
> 	* config/arc/arc.h (arc_return_address_regs): Delete prototype.
> 	(arc_function_type): Change encoding, add naked type.
> 	(ARC_INTERRUPT_P): Change to handle the new encoding.
> 	(ARC_FAST_INTERRUPT_P): Likewise.
> 	(ARC_NORMAL_P): Define.
> 	(ARC_NAKED_P): Likewise.
> 	(arc_compute_function_type): Delete prototype.
> 	* config/arc/arc.md (in_ret_delay_slot): Use
> 	arc_return_address_register function.
> 	(simple_return): Likewise.
> 	(p_return_i): Likewise.
> 
> gcc/testsuite
> 2016-12-13  Claudiu Zissulescu  <claziss@synopsys.com>
> 	    Andrew Burgess  <andrew.burgess@embecosm.com>
> 
> 	* gcc.target/arc/naked-1.c: New file.
> 	* gcc.target/arc/naked-2.c: Likewise.

Claudiu,

Sorry it's taken me a while to look at these patches.

I tried to apply this to the current GCC head, and it looks for me
like this doesn't apply.  Specifically `arc_expand_epilogue` does not
appear (in the current head) to have the code expected in this patch.

I have double checked at my end, but could you confirm that the patch
does apply cleanly for you please, then I'll spend some additional
time trying to figure out what I've done wrong :)

Thanks,
Andrew





> ---
>  gcc/config/arc/arc-protos.h            |   6 +-
>  gcc/config/arc/arc.c                   | 165 ++++++++++++++++++++++++---------
>  gcc/config/arc/arc.h                   |  40 +++++---
>  gcc/config/arc/arc.md                  |  10 +-
>  gcc/testsuite/gcc.target/arc/naked-1.c |  18 ++++
>  gcc/testsuite/gcc.target/arc/naked-2.c |  26 ++++++
>  6 files changed, 197 insertions(+), 68 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/arc/naked-1.c
>  create mode 100644 gcc/testsuite/gcc.target/arc/naked-2.c
> 
> diff --git a/gcc/config/arc/arc-protos.h b/gcc/config/arc/arc-protos.h
> index 4ff8e9b..b436dbe 100644
> --- a/gcc/config/arc/arc-protos.h
> +++ b/gcc/config/arc/arc-protos.h
> @@ -45,12 +45,10 @@ extern void arc_expand_atomic_op (enum rtx_code, rtx, rtx, rtx, rtx, rtx);
>  extern void arc_split_compare_and_swap (rtx *);
>  extern void arc_expand_compare_and_swap (rtx *);
>  extern bool compact_memory_operand_p (rtx, machine_mode, bool, bool);
> +extern int arc_return_address_register (unsigned int);
> +extern unsigned int arc_compute_function_type (struct function *);
>  #endif /* RTX_CODE */
>  
> -#ifdef TREE_CODE
> -extern enum arc_function_type arc_compute_function_type (struct function *);
> -#endif /* TREE_CODE */
> -
>  extern bool arc_ccfsm_branch_deleted_p (void);
>  extern void arc_ccfsm_record_branch_deleted (void);
>  
> diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> index a65fc3a..7dfc68e 100644
> --- a/gcc/config/arc/arc.c
> +++ b/gcc/config/arc/arc.c
> @@ -211,6 +211,7 @@ static int rgf_banked_register_count;
>  static int get_arc_condition_code (rtx);
>  
>  static tree arc_handle_interrupt_attribute (tree *, tree, tree, int, bool *);
> +static tree arc_handle_fndecl_attribute (tree *, tree, tree, int, bool *);
>  
>  /* Initialized arc_attribute_table to NULL since arc doesnot have any
>     machine specific supported attributes.  */
> @@ -229,6 +230,9 @@ const struct attribute_spec arc_attribute_table[] =
>    /* And these functions are always known to reside within the 21 bit
>       addressing range of blcc.  */
>    { "short_call",   0, 0, false, true,  true,  NULL, false },
> +  /* Function which are not having the prologue and epilogue generated
> +     by the compiler.  */
> +  { "naked", 0, 0, true, false, false, arc_handle_fndecl_attribute, false },
>    { NULL, 0, 0, false, false, false, NULL, false }
>  };
>  static int arc_comp_type_attributes (const_tree, const_tree);
> @@ -513,6 +517,12 @@ static void arc_finalize_pic (void);
>  #define TARGET_DIFFERENT_ADDR_DISPLACEMENT_P hook_bool_void_true
>  #define TARGET_SPILL_CLASS arc_spill_class
>  
> +#undef TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS
> +#define TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS arc_allocate_stack_slots_for_args
> +
> +#undef TARGET_WARN_FUNC_RETURN
> +#define TARGET_WARN_FUNC_RETURN arc_warn_func_return
> +
>  #include "target-def.h"
>  
>  #undef TARGET_ASM_ALIGNED_HI_OP
> @@ -1859,6 +1869,42 @@ arc_handle_interrupt_attribute (tree *, tree name, tree args, int,
>    return NULL_TREE;
>  }
>  
> +static tree
> +arc_handle_fndecl_attribute (tree *node, tree name, tree args ATTRIBUTE_UNUSED,
> +			     int flags ATTRIBUTE_UNUSED, bool *no_add_attrs)
> +{
> +  if (TREE_CODE (*node) != FUNCTION_DECL)
> +    {
> +      warning (OPT_Wattributes, "%qE attribute only applies to functions",
> +	       name);
> +      *no_add_attrs = true;
> +    }
> +
> +  return NULL_TREE;
> +}
> +
> +/* Implement `TARGET_ALLOCATE_STACK_SLOTS_FOR_ARGS' */
> +
> +static bool
> +arc_allocate_stack_slots_for_args (void)
> +{
> +  /* Naked functions should not allocate stack slots for arguments.  */
> +  unsigned int fn_type = arc_compute_function_type (cfun);
> +
> +  return !ARC_NAKED_P(fn_type);
> +}
> +
> +/* Implement `TARGET_WARN_FUNC_RETURN'.  */
> +
> +static bool
> +arc_warn_func_return (tree decl)
> +{
> +  struct function *func = DECL_STRUCT_FUNCTION (decl);
> +  unsigned int fn_type = arc_compute_function_type (func);
> +
> +  return !ARC_NAKED_P (fn_type);
> +}
> +
>  /* Return zero if TYPE1 and TYPE are incompatible, one if they are compatible,
>     and two if they are nearly compatible (which causes a warning to be
>     generated).  */
> @@ -2362,7 +2408,7 @@ struct GTY (()) arc_frame_info
>  
>  typedef struct GTY (()) machine_function
>  {
> -  enum arc_function_type fn_type;
> +  unsigned int fn_type;
>    struct arc_frame_info frame_info;
>    /* To keep track of unalignment caused by short insns.  */
>    int unalign;
> @@ -2380,43 +2426,40 @@ typedef struct GTY (()) machine_function
>     The result is cached.  To reset the cache at the end of a function,
>     call with DECL = NULL_TREE.  */
>  
> -enum arc_function_type
> +unsigned int
>  arc_compute_function_type (struct function *fun)
>  {
> -  tree decl = fun->decl;
> -  tree a;
> -  enum arc_function_type fn_type = fun->machine->fn_type;
> +  tree attr, decl = fun->decl;
> +  unsigned int fn_type = fun->machine->fn_type;
>  
>    if (fn_type != ARC_FUNCTION_UNKNOWN)
>      return fn_type;
>  
> -  /* Assume we have a normal function (not an interrupt handler).  */
> -  fn_type = ARC_FUNCTION_NORMAL;
> +  /* Check if it is a naked function.  */
> +  if (lookup_attribute ("naked", DECL_ATTRIBUTES (decl)) != NULL_TREE)
> +    fn_type |= ARC_FUNCTION_NAKED;
> +  else
> +    fn_type |= ARC_FUNCTION_NORMAL;
>  
>    /* Now see if this is an interrupt handler.  */
> -  for (a = DECL_ATTRIBUTES (decl);
> -       a;
> -       a = TREE_CHAIN (a))
> -    {
> -      tree name = TREE_PURPOSE (a), args = TREE_VALUE (a);
> -
> -      if (name == get_identifier ("interrupt")
> -	  && list_length (args) == 1
> -	  && TREE_CODE (TREE_VALUE (args)) == STRING_CST)
> -	{
> -	  tree value = TREE_VALUE (args);
> -
> -	  if (!strcmp (TREE_STRING_POINTER (value), "ilink1")
> -	      || !strcmp (TREE_STRING_POINTER (value), "ilink"))
> -	    fn_type = ARC_FUNCTION_ILINK1;
> -	  else if (!strcmp (TREE_STRING_POINTER (value), "ilink2"))
> -	    fn_type = ARC_FUNCTION_ILINK2;
> -	  else if (!strcmp (TREE_STRING_POINTER (value), "firq"))
> -	    fn_type = ARC_FUNCTION_FIRQ;
> -	  else
> -	    gcc_unreachable ();
> -	  break;
> -	}
> +  attr = lookup_attribute ("interrupt", DECL_ATTRIBUTES (decl));
> +  if (attr != NULL_TREE)
> +    {
> +      tree value, args = TREE_VALUE (attr);
> +
> +      gcc_assert (list_length (args) == 1);
> +      value = TREE_VALUE (args);
> +      gcc_assert (TREE_CODE (value) == STRING_CST);
> +
> +      if (!strcmp (TREE_STRING_POINTER (value), "ilink1")
> +	  || !strcmp (TREE_STRING_POINTER (value), "ilink"))
> +	fn_type |= ARC_FUNCTION_ILINK1;
> +      else if (!strcmp (TREE_STRING_POINTER (value), "ilink2"))
> +	fn_type |= ARC_FUNCTION_ILINK2;
> +      else if (!strcmp (TREE_STRING_POINTER (value), "firq"))
> +	fn_type |= ARC_FUNCTION_FIRQ;
> +      else
> +	gcc_unreachable ();
>      }
>  
>    return fun->machine->fn_type = fn_type;
> @@ -2436,7 +2479,7 @@ arc_compute_function_type (struct function *fun)
>  static bool
>  arc_must_save_register (int regno, struct function *func)
>  {
> -  enum arc_function_type fn_type = arc_compute_function_type (func);
> +  unsigned int fn_type = arc_compute_function_type (func);
>    bool irq_auto_save_p = ((irq_ctrl_saved.irq_save_last_reg >= regno)
>  			  && ARC_AUTO_IRQ_P (fn_type));
>    bool firq_auto_save_p = ARC_FAST_INTERRUPT_P (fn_type);
> @@ -2945,7 +2988,11 @@ arc_expand_prologue (void)
>       Change the stack layout so that we rather store a high register with the
>       PRE_MODIFY, thus enabling more short insn generation.)  */
>    int first_offset = 0;
> -  enum arc_function_type fn_type = arc_compute_function_type (cfun);
> +  unsigned int fn_type = arc_compute_function_type (cfun);
> +
> +  /* Naked functions don't have prologue.  */
> +  if (ARC_NAKED_P (fn_type))
> +      return;
>  
>    /* Compute total frame size.  */
>    size = arc_compute_frame_size ();
> @@ -3052,10 +3099,7 @@ void
>  arc_expand_epilogue (int sibcall_p)
>  {
>    int size;
> -  enum arc_function_type fn_type = arc_compute_function_type (cfun);
> -
> -  size = arc_compute_frame_size ();
> -
> +  unsigned int fn_type = arc_compute_function_type (cfun);
>    unsigned int pretend_size = cfun->machine->frame_info.pretend_size;
>    unsigned int frame_size;
>    unsigned int size_to_deallocate;
> @@ -3065,6 +3109,12 @@ arc_expand_epilogue (int sibcall_p)
>    int millicode_p = cfun->machine->frame_info.millicode_end_reg > 0;
>    rtx insn;
>  
> +  /* Naked functions don't have epilogue.  */
> +  if (ARC_NAKED_P (fn_type))
> +    return;
> +
> +  size = arc_compute_frame_size ();
> +
>    size_to_deallocate = size;
>  
>    frame_size = size - (pretend_size +
> @@ -9886,37 +9936,60 @@ arc_can_follow_jump (const rtx_insn *follower, const rtx_insn *followee)
>    return true;
>  }
>  
> -int arc_return_address_regs[5] =
> -  {0, RETURN_ADDR_REGNUM, ILINK1_REGNUM, ILINK2_REGNUM, ILINK1_REGNUM};
> +/* Return the register number of the register holding the return address
> +   for a function of type TYPE.  */
> +
> +int
> +arc_return_address_register (unsigned int fn_type)
> +{
> +  int regno = 0;
> +
> +  if (ARC_INTERRUPT_P (fn_type))
> +    {
> +      if (((fn_type & ARC_FUNCTION_ILINK1) | ARC_FUNCTION_FIRQ) != 0)
> +        regno = ILINK1_REGNUM;
> +      else if ((fn_type & ARC_FUNCTION_ILINK2) != 0)
> +        regno = ILINK2_REGNUM;
> +      else
> +        gcc_unreachable ();
> +    }
> +  else if (ARC_NORMAL_P (fn_type) || ARC_NAKED_P (fn_type))
> +    regno = RETURN_ADDR_REGNUM;
>  
> -/* Implement EPILOGUE__USES.
> +  gcc_assert (regno != 0);
> +  return regno;
> +}
> +
> +/* Implement EPILOGUE_USES.
>     Return true if REGNO should be added to the deemed uses of the epilogue.
>  
> -   We use the return address
> -   arc_return_address_regs[arc_compute_function_type (cfun)].  But
> -   also, we have to make sure all the register restore instructions
> -   are known to be live in interrupt functions, plus the blink
> -   register if it is clobbered by the isr.  */
> +   We have to make sure all the register restore instructions are
> +   known to be live in interrupt functions, plus the blink register if
> +   it is clobbered by the isr.  */
>  
>  bool
>  arc_epilogue_uses (int regno)
>  {
> +  unsigned int fn_type;
> +
>    if (regno == arc_tp_regno)
>      return true;
> +
> +  fn_type = arc_compute_function_type (cfun);
>    if (reload_completed)
>      {
>        if (ARC_INTERRUPT_P (cfun->machine->fn_type))
>  	{
>  	  if (!fixed_regs[regno])
>  	    return true;
> -	  return ((regno == arc_return_address_regs[cfun->machine->fn_type])
> +	  return ((regno == arc_return_address_register (fn_type))
>  		  || (regno == RETURN_ADDR_REGNUM));
>  	}
>        else
>  	return regno == RETURN_ADDR_REGNUM;
>      }
>    else
> -    return regno == arc_return_address_regs[arc_compute_function_type (cfun)];
> +    return regno == arc_return_address_register (fn_type);
>  }
>  
>  /* Helper for EH_USES macro.  */
> diff --git a/gcc/config/arc/arc.h b/gcc/config/arc/arc.h
> index fbc1195..16d5319 100644
> --- a/gcc/config/arc/arc.h
> +++ b/gcc/config/arc/arc.h
> @@ -1363,10 +1363,6 @@ do { \
>  #define ASM_OUTPUT_ALIGNED_DECL_LOCAL(STREAM, DECL, NAME, SIZE, ALIGNMENT) \
>    arc_asm_output_aligned_decl_local (STREAM, DECL, NAME, SIZE, ALIGNMENT, 0)
>  
> -/* To translate the return value of arc_function_type into a register number
> -   to jump through for function return.  */
> -extern int arc_return_address_regs[5];
> -
>  /* Debugging information.  */
>  
>  /* Generate DBX and DWARF debugging information.  */
> @@ -1502,22 +1498,38 @@ extern struct rtx_def *arc_compare_op0, *arc_compare_op1;
>  
>  /* ARC function types.   */
>  enum arc_function_type {
> -  ARC_FUNCTION_UNKNOWN, ARC_FUNCTION_NORMAL,
> +  /* No function should have the unknown type.  This value is used to
> +   indicate the that function type has not yet been computed.  */
> +  ARC_FUNCTION_UNKNOWN  = 0,
> +
> +  /* The normal function type indicates that the function has the
> +   standard prologue and epilogue.  */
> +  ARC_FUNCTION_NORMAL  = 1 << 0,
>    /* These are interrupt handlers.  The name corresponds to the register
>       name that contains the return address.  */
> -  ARC_FUNCTION_ILINK1, ARC_FUNCTION_ILINK2,
> +  ARC_FUNCTION_ILINK1  = 1 << 1,
> +  ARC_FUNCTION_ILINK2  = 1 << 2,
>    /* Fast interrupt is only available on ARCv2 processors.  */
> -  ARC_FUNCTION_FIRQ
> +  ARC_FUNCTION_FIRQ    = 1 << 3,
> +  /* The naked function type indicates that the function does not have
> +   prologue or epilogue, and that no stack frame is available.  */
> +  ARC_FUNCTION_NAKED   = 1 << 4
>  };
> -#define ARC_INTERRUPT_P(TYPE)						\
> -  (((TYPE) == ARC_FUNCTION_ILINK1) || ((TYPE) == ARC_FUNCTION_ILINK2)	\
> -   || ((TYPE) == ARC_FUNCTION_FIRQ))
>  
> -#define ARC_FAST_INTERRUPT_P(TYPE) ((TYPE) == ARC_FUNCTION_FIRQ)
> +/* Check if a function is an interrupt function.  */
> +#define ARC_INTERRUPT_P(TYPE)					\
> +  (((TYPE) & (ARC_FUNCTION_ILINK1 | ARC_FUNCTION_ILINK2		\
> +	      | ARC_FUNCTION_FIRQ)) != 0)
> +
> +/* Check if a function is a fast interrupt function.  */
> +#define ARC_FAST_INTERRUPT_P(TYPE) (((TYPE) & ARC_FUNCTION_FIRQ) != 0)
> +
> +/* Check if a function is normal, that is, has standard prologue and
> +   epilogue.  */
> +#define ARC_NORMAL_P(TYPE) (((TYPE) & ARC_FUNCTION_NORMAL) != 0)
>  
> -/* Compute the type of a function from its DECL.  Needed for EPILOGUE_USES.  */
> -struct function;
> -extern enum arc_function_type arc_compute_function_type (struct function *);
> +/* Check if a function is naked.  */
> +#define ARC_NAKED_P(TYPE) (((TYPE) & ARC_FUNCTION_NAKED) != 0)
>  
>  /* Called by crtstuff.c to make calls to function FUNCTION that are defined in
>     SECTION_OP, and then to switch back to text section.  */
> diff --git a/gcc/config/arc/arc.md b/gcc/config/arc/arc.md
> index 9a97490..39bcc26 100644
> --- a/gcc/config/arc/arc.md
> +++ b/gcc/config/arc/arc.md
> @@ -505,8 +505,8 @@
>    (cond [(eq_attr "in_delay_slot" "false")
>  	 (const_string "no")
>  	 (match_test "regno_clobbered_p
> -			(arc_return_address_regs
> -			  [arc_compute_function_type (cfun)],
> +			(arc_return_address_register
> +			  (arc_compute_function_type (cfun)),
>  			 insn, SImode, 1)")
>  	 (const_string "no")]
>  	(const_string "yes")))
> @@ -4859,7 +4859,8 @@
>  {
>    rtx reg
>      = gen_rtx_REG (Pmode,
> -		   arc_return_address_regs[arc_compute_function_type (cfun)]);
> +		   arc_return_address_register (arc_compute_function_type
> +						(cfun)));
>  
>    if (TARGET_V2
>        && ARC_INTERRUPT_P (arc_compute_function_type (cfun)))
> @@ -4908,7 +4909,8 @@
>    xop[0] = operands[0];
>    xop[1]
>      = gen_rtx_REG (Pmode,
> -		   arc_return_address_regs[arc_compute_function_type (cfun)]);
> +		   arc_return_address_register (arc_compute_function_type
> +						(cfun)));
>  
>    if (TARGET_PAD_RETURN)
>      arc_pad_return ();
> diff --git a/gcc/testsuite/gcc.target/arc/naked-1.c b/gcc/testsuite/gcc.target/arc/naked-1.c
> new file mode 100644
> index 0000000..e45f433f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/naked-1.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O0" } */
> +/* Check that naked functions don't place arguments on the stack at
> +   optimisation level '-O0'.  */
> +extern void bar (int);
> +
> +void __attribute__((naked))
> +foo (int n, int m)
> +{
> +  bar (n + m);
> +}
> +/* { dg-final { scan-assembler "\tbl @bar" } } */
> +
> +/* Look for things that would appear in a non-naked function, but which
> +   should not appear in a naked function.  */
> +/* { dg-final { scan-assembler-not "\tj.* \\\[blink\\\]" } } */
> +/* { dg-final { scan-assembler-not "\tst.* " } } */
> +/* { dg-final { scan-assembler-not "\tmov fp,sp" } } */
> diff --git a/gcc/testsuite/gcc.target/arc/naked-2.c b/gcc/testsuite/gcc.target/arc/naked-2.c
> new file mode 100644
> index 0000000..7b7262f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arc/naked-2.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O0" } */
> +/* Check that naked functions don't place arguments on the stack at
> +   optimisation level '-O0'.  */
> +
> +#if defined(__HS__) || defined(__EM__)
> +# define ILINK "ilink"
> +#else
> +# define ILINK "ilink1"
> +#endif
> +
> +extern void bar (int);
> +
> +void __attribute__((naked, interrupt(ILINK)))
> +foo (int n, int m)
> +{
> +  bar (n + m);
> +}
> +/* { dg-final { scan-assembler "\tbl @bar" } } */
> +
> +/* Look for things that would appear in a non-naked function, but which
> +   should not appear in a naked function.  */
> +/* { dg-final { scan-assembler-not "\trtie" } } */
> +/* { dg-final { scan-assembler-not "j.*\[ilink1\]" } } */
> +/* { dg-final { scan-assembler-not "\tst.* " } } */
> +/* { dg-final { scan-assembler-not "\tmov fp,sp" } } */
> -- 
> 1.9.1
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]