From: Pedro Alves <palves@redhat.com>
To: Alan Hayward <alan.hayward@arm.com>, gdb-patches@sourceware.org
Cc: nd@arm.com
Subject: Re: [PATCH v3 1/3] Use enum for return method for dummy calls
Date: Fri, 19 Oct 2018 11:28:00 -0000 [thread overview]
Message-ID: <d47924af-1f91-25ad-3691-43ccc2fc62ae@redhat.com> (raw)
In-Reply-To: <20181011144905.66908-2-alan.hayward@arm.com>
On 10/11/2018 03:49 PM, Alan Hayward wrote:
> In call_function_by_hand_dummy, struct_return and hidden_first_param_p
> are used to represent a single concept. Replace with an enum.
>
> gdb/ChangeLog:
>
> 2018-10-11 Alan Hayward <alan.hayward@arm.com>
>
> * gdbarch.sh (enum function_call_return_method): Add enum.
> * gdbarch.h: Regenerate.
> * infcall.c (call_function_by_hand_dummy): Replace vars with enum.
> ---
> gdb/gdbarch.h | 17 +++++++++++++++++
> gdb/gdbarch.sh | 17 +++++++++++++++++
> gdb/infcall.c | 29 +++++++++++------------------
> 3 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
> index fc2f1a84a1..5f9cf481fb 100644
> --- a/gdb/gdbarch.h
> +++ b/gdb/gdbarch.h
> @@ -102,6 +102,23 @@ typedef void (iterate_over_regset_sections_cb)
> (const char *sect_name, int supply_size, int collect_size,
> const struct regset *regset, const char *human_name, void *cb_data);
>
> +/* For a function call, are we returning a value using a normal value return
> + or a structure return - passing a hidden argument pointing to storage.
Pedantically, this is not a property of the call action that
we're performing, but a property of the function's call ABI.
I'd suggest tweaking the sentence like this:
/* For a function call, does the function return a value using a
normal value return or a structure return - passing a hidden
argument pointing to storage.
> + There are two cases: language-mandated structure return and target ABI
> + structure return.
That "There are two cases" is a bit confusing, because it makes you think
about normal vs not-normal. So I'd suggest starting with "For the latter", like:
For the latter, there are two cases: language-mandated structure
return and target ABI [....]
The language version is handled by passing the return
> + location as the first parameter to the function, even preceding "this".
> + This is different from the target ABI version, which is target-specific; for
> + instance, on ia64 the first argument is passed in out0 but the hidden
> + structure return pointer would normally be passed in r8. */
> +
> +enum function_call_return_method
> +{
> + return_method_normal = 0, /* Standard value return. */
> + return_method_struct, /* target ABI structure return. */
Uppercase "target" -> "Target".
> + return_method_hidden_param /* Return hidden in first param. */
These last two are confusing if you consider the comment in the intro
that says:
"or a structure return - passing a hidden argument pointing to storage."
I.e., as is, makes you wonder whether "return_method_struct" is
or is not a hidden param too?
> +};
> +
Keeping the enum/enumerators names, how about this:
/* For a function call, does the function return a value using a
normal value return or a structure return - passing a hidden
argument pointing to storage. For the latter, there are two
cases: language-mandated structure return and target ABI
structure return. */
enum function_call_return_method
{
/* Standard value return. */
return_method_normal = 0,
/* Language ABI structure return. This is handled
by passing the return location as the first parameter to
the function, even preceding "this".
return_method_hidden_param,
/* Target ABI struct return. This is target-specific; for instance,
on ia64 the first argument is passed in out0 but the hidden
structure return pointer would normally be passed in r8. */
return_method_struct,
};
Thanks,
Pedro Alves
next prev parent reply other threads:[~2018-10-19 11:28 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-11 14:49 [PATCH v3 0/3] Aarch64: Fix segfault when casting " Alan Hayward
2018-10-11 14:49 ` [PATCH v3 3/3] " Alan Hayward
2018-10-19 11:36 ` Pedro Alves
2018-10-23 16:08 ` Alan Hayward
2018-10-24 15:15 ` Pedro Alves
2018-10-29 11:58 ` Alan Hayward
2018-10-29 12:38 ` Pedro Alves
2018-10-29 14:56 ` Alan Hayward
2018-10-29 18:13 ` Pedro Alves
2018-10-30 11:13 ` Alan Hayward
2018-10-30 16:31 ` Pedro Alves
2018-10-30 17:09 ` Alan Hayward
2018-10-30 17:40 ` Pedro Alves
2018-10-11 14:49 ` [PATCH v3 2/3] Pass return_method to _push_dummy_call Alan Hayward
2018-10-19 11:31 ` Pedro Alves
2018-10-11 14:49 ` [PATCH v3 1/3] Use enum for return method for dummy calls Alan Hayward
2018-10-19 11:28 ` Pedro Alves [this message]
2018-10-18 9:50 ` [PING][PATCH v3 0/3] Aarch64: Fix segfault when casting " Alan Hayward
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d47924af-1f91-25ad-3691-43ccc2fc62ae@redhat.com \
--to=palves@redhat.com \
--cc=alan.hayward@arm.com \
--cc=gdb-patches@sourceware.org \
--cc=nd@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).