[PATCH 2/3] [amdgcn] Use frame pointer for CFA expressions.

[Andrew Stubbs]

On 22/06/2021 18:14, Hafiz Abid Qadeer wrote:
> As size of address is bigger than registers in amdgcn, we are forced to use
> DW_CFA_def_cfa_expression to make an expression that concatenates multiple
> registers for the value of the CFA.  This then prohibits us from using many
> of the dwarf ops which expect CFA rule to be a single regsiter plus an offset.
> Using frame pointer in the CFA rule is only real possibility as it is saved
> in every frame and it is easy to unwind its value.
> 
> So unless user gives fomit-frame-pointer, we use frame pointer for the
> cfi information.  This options also has a different default now.
> 
> gcc/
> 
> 	* common/config/gcn/gcn-common.c
> 	(gcn_option_optimization_table): Change OPT_fomit_frame_pointer to -O3.
> 	(gcn_expand_prologue): Prefer the frame pointer when emitting CFI.
> 	(gcn_frame_pointer_rqd): New function.
> 	(TARGET_FRAME_POINTER_REQUIRED): New hook.
> ---
>   gcc/common/config/gcn/gcn-common.c |  2 +-
>   gcc/config/gcn/gcn.c               | 60 +++++++++++++++++++++++-------
>   2 files changed, 47 insertions(+), 15 deletions(-)
> 
> diff --git a/gcc/common/config/gcn/gcn-common.c b/gcc/common/config/gcn/gcn-common.c
> index 305c310f940..695eb467e34 100644
> --- a/gcc/common/config/gcn/gcn-common.c
> +++ b/gcc/common/config/gcn/gcn-common.c
> @@ -27,7 +27,7 @@
>   /* Set default optimization options.  */
>   static const struct default_options gcn_option_optimization_table[] =
>     {
> -    { OPT_LEVELS_1_PLUS, OPT_fomit_frame_pointer, NULL, 1 },
> +    { OPT_LEVELS_3_PLUS, OPT_fomit_frame_pointer, NULL, 1 },
>       { OPT_LEVELS_NONE, 0, NULL, 0 }
>     };
>   
> diff --git a/gcc/config/gcn/gcn.c b/gcc/config/gcn/gcn.c
> index 3ab16548aad..0eac3aa3844 100644
> --- a/gcc/config/gcn/gcn.c
> +++ b/gcc/config/gcn/gcn.c
> @@ -2900,10 +2900,14 @@ gcn_expand_prologue ()
>   	  rtx adjustment = gen_int_mode (sp_adjust, SImode);
>   	  rtx insn = emit_insn (gen_addsi3_scalar_carry (sp_lo, sp_lo,
>   							 adjustment, scc));
> -	  RTX_FRAME_RELATED_P (insn) = 1;
> -	  add_reg_note (insn, REG_FRAME_RELATED_EXPR,
> -			gen_rtx_SET (sp,
> -				     gen_rtx_PLUS (DImode, sp, adjustment)));
> +	  if (!offsets->need_frame_pointer)
> +	    {
> +	      RTX_FRAME_RELATED_P (insn) = 1;
> +	      add_reg_note (insn, REG_FRAME_RELATED_EXPR,
> +			    gen_rtx_SET (sp,
> +					 gen_rtx_PLUS (DImode, sp,
> +						       adjustment)));
> +	    }
>   	  emit_insn (gen_addcsi3_scalar_zero (sp_hi, sp_hi, scc));
>   	}
>   
> @@ -2917,25 +2921,24 @@ gcn_expand_prologue ()
>   	  rtx adjustment = gen_int_mode (fp_adjust, SImode);
>   	  rtx insn = emit_insn (gen_addsi3_scalar_carry(fp_lo, sp_lo,
>   							adjustment, scc));
> -	  RTX_FRAME_RELATED_P (insn) = 1;
> -	  add_reg_note (insn, REG_FRAME_RELATED_EXPR,
> -			gen_rtx_SET (fp,
> -				     gen_rtx_PLUS (DImode, sp, adjustment)));
>   	  emit_insn (gen_addcsi3_scalar (fp_hi, sp_hi,
>   					 (fp_adjust < 0 ? GEN_INT (-1)
>   					  : const0_rtx),
>   					 scc, scc));
> +
> +	  /* Set the CFA to the entry stack address, as an offset from the
> +	     frame pointer.  This is preferred because the frame pointer is
> +	     saved in each frame, whereas the stack pointer is not.  */
> +	  RTX_FRAME_RELATED_P (insn) = 1;
> +	  add_reg_note (insn, REG_CFA_DEF_CFA,
> +			gen_rtx_PLUS (DImode, fp,
> +				      GEN_INT (-(offsets->pretend_size
> +						 + offsets->callee_saves))));
>   	}
>   
>         rtx_insn *seq = get_insns ();
>         end_sequence ();
>   
> -      /* FIXME: Prologue insns should have this flag set for debug output, etc.
> -	 but it causes issues for now.
> -      for (insn = seq; insn; insn = NEXT_INSN (insn))
> -        if (INSN_P (insn))
> -	  RTX_FRAME_RELATED_P (insn) = 1;*/
> -
>         emit_insn (seq);
>       }
>     else
> @@ -3011,6 +3014,16 @@ gcn_expand_prologue ()
>   		    gen_rtx_SET (sp, gen_rtx_PLUS (DImode, sp,
>   						   dbg_adjustment)));
>   
> +      if (offsets->need_frame_pointer)
> +	{
> +	  /* Set the CFA to the entry stack address, as an offset from the
> +	     frame pointer.  This is necessary when alloca is used, and
> +	     harmless otherwise.  */
> +	  rtx neg_adjust = gen_int_mode (-offsets->callee_saves, DImode);
> +	  add_reg_note (insn, REG_CFA_DEF_CFA,
> +			gen_rtx_PLUS (DImode, fp, neg_adjust));
> +	}
> +
>         /* Make sure the flat scratch reg doesn't get optimised away.  */
>         emit_insn (gen_prologue_use (gen_rtx_REG (DImode, FLAT_SCRATCH_REG)));
>       }
> @@ -3114,6 +3127,23 @@ gcn_expand_epilogue (void)
>     emit_jump_insn (gen_gcn_return ());
>   }
>   
> +/* Implement TARGET_FRAME_POINTER_REQUIRED.
> +
> +   Return true if the frame pointer should not be eliminated.  */
> +
> +bool
> +gcn_frame_pointer_rqd (void)
> +{
> +  /* GDB needs the frame pointer in order to unwind properly,
> +     but that's not important for the entry point, unless alloca is used.
> +     It's not important for code execution, so we should repect the
> +     -fomit-frame-pointer flag.  */
> +  return (!flag_omit_frame_pointer
> +	  && cfun
> +	  && (cfun->calls_alloca
> +	      || (cfun->machine && cfun->machine->normal_function)));
> +}
> +
>   /* Implement TARGET_CAN_ELIMINATE.
>    
>      Return true if the compiler is allowed to try to replace register number
> @@ -6373,6 +6403,8 @@ gcn_dwarf_register_span (rtx rtl)
>   #define TARGET_EMUTLS_VAR_INIT gcn_emutls_var_init
>   #undef  TARGET_EXPAND_BUILTIN
>   #define TARGET_EXPAND_BUILTIN gcn_expand_builtin
> +#undef  TARGET_FRAME_POINTER_REQUIRED
> +#define TARGET_FRAME_POINTER_REQUIRED gcn_frame_pointer_rqd
>   #undef  TARGET_FUNCTION_ARG
>   #undef  TARGET_FUNCTION_ARG_ADVANCE
>   #define TARGET_FUNCTION_ARG_ADVANCE gcn_function_arg_advance
> 

OK.

Andrew