[v3,#1/2] enable adjustment of return_pc debug attrs

On Apr 27, 2023, Alexandre Oliva <oliva@adacore.com> wrote:

> On Apr 14, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>> On Mar 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>> This patch introduces infrastructure for targets to add an offset to
>>> the label issued after the call_insn to set the call_return_pc
>>> attribute.  This will be used on rs6000, that sometimes issues another
>>> instruction after the call proper as part of a call insn.

>> Ping?
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614452.html

Ping?
Refreshed, retested on ppc64le-linux-gnu.  Ok to install?


This patch introduces infrastructure for targets to add an offset to
the label issued after the call_insn to set the call_return_pc
attribute.  This will be used on rs6000, that sometimes issues another
instruction after the call proper as part of a call insn.


for  gcc/ChangeLog

	* target.def (call_offset_return_label): New hook.
	* gcc/doc/tm.texi.in (TARGET_CALL_OFFSET_RETURN_LABEL): Add
	placeholder.
	* gcc/doc/tm.texi: Rebuild.
	* dwarf2out.cc (struct call_arg_loc_node): Record call_insn
	instad of call_arg_loc_note.
	(add_AT_lbl_id): Add optional offset argument.
	(gen_call_site_die): Compute and pass on a return pc offset.
	(gen_subprogram_die): Move call_arg_loc_note computation...
	(dwarf2out_var_location): ... from here.  Set call_insn.
---
 gcc/doc/tm.texi    |    7 +++++++
 gcc/doc/tm.texi.in |    2 ++
 gcc/dwarf2out.cc   |   26 +++++++++++++++++---------
 gcc/target.def     |    9 +++++++++
 4 files changed, 35 insertions(+), 9 deletions(-)

On 5/25/24 08:12, Alexandre Oliva wrote:
> On Apr 27, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>> On Apr 14, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>> On Mar 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>>> This patch introduces infrastructure for targets to add an offset to
>>>> the label issued after the call_insn to set the call_return_pc
>>>> attribute.  This will be used on rs6000, that sometimes issues another
>>>> instruction after the call proper as part of a call insn.
> 
>>> Ping?
>>> https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614452.html
> 
> Ping?
> Refreshed, retested on ppc64le-linux-gnu.  Ok to install?

I wonder about adding this information to REG_CALL_ARG_LOCATION, but 
doing it this way also seems reasonable.  I'm interested in Jakub's 
input, but the patch is OK in a week if he doesn't get to it.
> This patch introduces infrastructure for targets to add an offset to
> the label issued after the call_insn to set the call_return_pc
> attribute.  This will be used on rs6000, that sometimes issues another
> instruction after the call proper as part of a call insn.
> 
> 
> for  gcc/ChangeLog
> 
> 	* target.def (call_offset_return_label): New hook.
> 	* gcc/doc/tm.texi.in (TARGET_CALL_OFFSET_RETURN_LABEL): Add
> 	placeholder.
> 	* gcc/doc/tm.texi: Rebuild.
> 	* dwarf2out.cc (struct call_arg_loc_node): Record call_insn
> 	instad of call_arg_loc_note.
> 	(add_AT_lbl_id): Add optional offset argument.
> 	(gen_call_site_die): Compute and pass on a return pc offset.
> 	(gen_subprogram_die): Move call_arg_loc_note computation...
> 	(dwarf2out_var_location): ... from here.  Set call_insn.
> ---
>   gcc/doc/tm.texi    |    7 +++++++
>   gcc/doc/tm.texi.in |    2 ++
>   gcc/dwarf2out.cc   |   26 +++++++++++++++++---------
>   gcc/target.def     |    9 +++++++++
>   4 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
> index cd50078227d98..8a7aa70d605ba 100644
> --- a/gcc/doc/tm.texi
> +++ b/gcc/doc/tm.texi
> @@ -5557,6 +5557,13 @@ except the last are treated as named.
>   You need not define this hook if it always returns @code{false}.
>   @end deftypefn
>   
> +@deftypefn {Target Hook} int TARGET_CALL_OFFSET_RETURN_LABEL (rtx_insn *@var{call_insn})
> +While generating call-site debug info for a CALL insn, or a SEQUENCE
> +insn starting with a CALL, this target hook is invoked to compute the
> +offset to be added to the debug label emitted after the call to obtain
> +the return address that should be recorded as the return PC.
> +@end deftypefn
> +
>   @deftypefn {Target Hook} void TARGET_START_CALL_ARGS (cumulative_args_t @var{complete_args})
>   This target hook is invoked while generating RTL for a function call,
>   after the argument values have been computed, and after stack arguments
> diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
> index 058bd56487a9a..9e0830758aeea 100644
> --- a/gcc/doc/tm.texi.in
> +++ b/gcc/doc/tm.texi.in
> @@ -3887,6 +3887,8 @@ These machine description macros help implement varargs:
>   
>   @hook TARGET_STRICT_ARGUMENT_NAMING
>   
> +@hook TARGET_CALL_OFFSET_RETURN_LABEL
> +
>   @hook TARGET_START_CALL_ARGS
>   
>   @hook TARGET_CALL_ARGS
> diff --git a/gcc/dwarf2out.cc b/gcc/dwarf2out.cc
> index 5b064ffd78ad1..1092880738df4 100644
> --- a/gcc/dwarf2out.cc
> +++ b/gcc/dwarf2out.cc
> @@ -3593,7 +3593,7 @@ typedef struct var_loc_list_def var_loc_list;
>   
>   /* Call argument location list.  */
>   struct GTY ((chain_next ("%h.next"))) call_arg_loc_node {
> -  rtx GTY (()) call_arg_loc_note;
> +  rtx_insn * GTY (()) call_insn;
>     const char * GTY (()) label;
>     tree GTY (()) block;
>     bool tail_call_p;
> @@ -3777,7 +3777,8 @@ static void remove_addr_table_entry (addr_table_entry *);
>   static void add_AT_addr (dw_die_ref, enum dwarf_attribute, rtx, bool);
>   static inline rtx AT_addr (dw_attr_node *);
>   static void add_AT_symview (dw_die_ref, enum dwarf_attribute, const char *);
> -static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *);
> +static void add_AT_lbl_id (dw_die_ref, enum dwarf_attribute, const char *,
> +			   int = 0);
>   static void add_AT_lineptr (dw_die_ref, enum dwarf_attribute, const char *);
>   static void add_AT_macptr (dw_die_ref, enum dwarf_attribute, const char *);
>   static void add_AT_range_list (dw_die_ref, enum dwarf_attribute,
> @@ -5353,14 +5354,17 @@ add_AT_symview (dw_die_ref die, enum dwarf_attribute attr_kind,
>   
>   static inline void
>   add_AT_lbl_id (dw_die_ref die, enum dwarf_attribute attr_kind,
> -               const char *lbl_id)
> +	       const char *lbl_id, int offset)
>   {
>     dw_attr_node attr;
>   
>     attr.dw_attr = attr_kind;
>     attr.dw_attr_val.val_class = dw_val_class_lbl_id;
>     attr.dw_attr_val.val_entry = NULL;
> -  attr.dw_attr_val.v.val_lbl_id = xstrdup (lbl_id);
> +  if (!offset)
> +    attr.dw_attr_val.v.val_lbl_id = xstrdup (lbl_id);
> +  else
> +    attr.dw_attr_val.v.val_lbl_id = xasprintf ("%s%+i", lbl_id, offset);
>     if (dwarf_split_debug_info)
>       attr.dw_attr_val.val_entry
>           = add_addr_table_entry (attr.dw_attr_val.v.val_lbl_id,
> @@ -23515,7 +23519,9 @@ gen_call_site_die (tree decl, dw_die_ref subr_die,
>     if (stmt_die == NULL)
>       stmt_die = subr_die;
>     die = new_die (dwarf_TAG (DW_TAG_call_site), stmt_die, NULL_TREE);
> -  add_AT_lbl_id (die, dwarf_AT (DW_AT_call_return_pc), ca_loc->label);
> +  add_AT_lbl_id (die, dwarf_AT (DW_AT_call_return_pc),
> +		 ca_loc->label,
> +		 targetm.calls.call_offset_return_label (ca_loc->call_insn));
>     if (ca_loc->tail_call_p)
>       add_AT_flag (die, dwarf_AT (DW_AT_call_tail_call), 1);
>     if (ca_loc->symbol_ref)
> @@ -24202,11 +24208,14 @@ gen_subprogram_die (tree decl, dw_die_ref context_die)
>   	    {
>   	      dw_die_ref die = NULL;
>   	      rtx tloc = NULL_RTX, tlocc = NULL_RTX;
> +	      rtx call_arg_loc_note
> +		= find_reg_note (ca_loc->call_insn,
> +				 REG_CALL_ARG_LOCATION, NULL_RTX);
>   	      rtx arg, next_arg;
>   	      tree arg_decl = NULL_TREE;
>   
> -	      for (arg = (ca_loc->call_arg_loc_note != NULL_RTX
> -			  ? XEXP (ca_loc->call_arg_loc_note, 0)
> +	      for (arg = (call_arg_loc_note != NULL_RTX
> +			  ? XEXP (call_arg_loc_note, 0)
>   			  : NULL_RTX);
>   		   arg; arg = next_arg)
>   		{
> @@ -28251,8 +28260,7 @@ create_label:
>   	= ggc_cleared_alloc<call_arg_loc_node> ();
>         rtx_insn *prev = call_insn;
>   
> -      ca_loc->call_arg_loc_note
> -	= find_reg_note (call_insn, REG_CALL_ARG_LOCATION, NULL_RTX);
> +      ca_loc->call_insn = call_insn;
>         ca_loc->next = NULL;
>         ca_loc->label = last_label;
>         gcc_assert (prev
> diff --git a/gcc/target.def b/gcc/target.def
> index c27df8095be15..70070caebc768 100644
> --- a/gcc/target.def
> +++ b/gcc/target.def
> @@ -4890,6 +4890,15 @@ Most ports do not need to implement anything for this hook.",
>    void, (cumulative_args_t complete_args),
>    hook_void_CUMULATIVE_ARGS)
>   
> +DEFHOOK
> +(call_offset_return_label,
> + "While generating call-site debug info for a CALL insn, or a SEQUENCE\n\
> +insn starting with a CALL, this target hook is invoked to compute the\n\
> +offset to be added to the debug label emitted after the call to obtain\n\
> +the return address that should be recorded as the return PC.",
> + int, (rtx_insn *call_insn),
> + hook_int_rtx_insn_0)
> +
>   DEFHOOK
>   (push_argument,
>    "This target hook returns @code{true} if push instructions will be\n\
> 
>

On Sat, May 25, 2024 at 09:12:05AM -0300, Alexandre Oliva wrote:
<snip>

You sent multiple patch series in one thread, and multiple versions of
the same series even.

This is very hard to even *read*, let alone work with.  Please don't.


Segher

On May 28, 2024, Jason Merrill <jason@redhat.com> wrote:

> On 5/25/24 08:12, Alexandre Oliva wrote:
>> On Apr 27, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>> On Apr 14, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>>> On Mar 23, 2023, Alexandre Oliva <oliva@adacore.com> wrote:
>>>>> This patch introduces infrastructure for targets to add an offset to
>>>>> the label issued after the call_insn to set the call_return_pc
>>>>> attribute.  This will be used on rs6000, that sometimes issues another
>>>>> instruction after the call proper as part of a call insn.
>> 
>>>> Ping?
>>>> https://gcc.gnu.org/pipermail/gcc-patches/2023-March/614452.html
>> Ping?
>> Refreshed, retested on ppc64le-linux-gnu.  Ok to install?

> I wonder about adding this information to REG_CALL_ARG_LOCATION, but
> doing it this way also seems reasonable.  I'm interested in Jakub's 
> input, but the patch is OK in a week if he doesn't get to it.

Thanks, I'm putting it in, but I also look forward to Jakub's feedback.

As for REG_CALL_ARG_LOCATION, I suppose that would be a decent place to
hold it for the new hook to get at it, but since it can usually be
computed directly, possibly with help of an attribute, adding extra rtl
to call insns is probably unnecessary and undesirable.

>> for  gcc/ChangeLog
>> * target.def (call_offset_return_label): New hook.
>> * gcc/doc/tm.texi.in (TARGET_CALL_OFFSET_RETURN_LABEL): Add
>> placeholder.
>> * gcc/doc/tm.texi: Rebuild.
>> * dwarf2out.cc (struct call_arg_loc_node): Record call_insn
>> instad of call_arg_loc_note.
>> (add_AT_lbl_id): Add optional offset argument.
>> (gen_call_site_die): Compute and pass on a return pc offset.
>> (gen_subprogram_die): Move call_arg_loc_note computation...
>> (dwarf2out_var_location): ... from here.  Set call_insn.