From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 122195 invoked by alias); 19 Oct 2018 11:28:34 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 122169 invoked by uid 89); 19 Oct 2018 11:28:33 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=pedantically, uppercase X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 19 Oct 2018 11:28:32 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 44B2E76540; Fri, 19 Oct 2018 11:28:31 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id 53E591A7ED; Fri, 19 Oct 2018 11:28:30 +0000 (UTC) Subject: Re: [PATCH v3 1/3] Use enum for return method for dummy calls To: Alan Hayward , gdb-patches@sourceware.org References: <20181011144905.66908-1-alan.hayward@arm.com> <20181011144905.66908-2-alan.hayward@arm.com> Cc: nd@arm.com From: Pedro Alves Message-ID: Date: Fri, 19 Oct 2018 11:28:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181011144905.66908-2-alan.hayward@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2018-10/txt/msg00425.txt.bz2 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 > > * 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