From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64544 invoked by alias); 28 Aug 2018 15:58:20 -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 64534 invoked by uid 89); 28 Aug 2018 15:58:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=*function, our X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 28 Aug 2018 15:58:18 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w7SFwB9j014580 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 28 Aug 2018 11:58:16 -0400 Received: by simark.ca (Postfix, from userid 112) id 897F01E186; Tue, 28 Aug 2018 11:58:11 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 0333D1E05F; Tue, 28 Aug 2018 11:58:10 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 28 Aug 2018 15:58:00 -0000 From: Simon Marchi To: Alan Hayward Cc: gdb-patches@sourceware.org, nd@arm.com Subject: Re: [PATCH 2/4] Aarch64: Float register detection for _push_dummy_call In-Reply-To: <20180820092933.83224-3-alan.hayward@arm.com> References: <20180820092933.83224-1-alan.hayward@arm.com> <20180820092933.83224-3-alan.hayward@arm.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00688.txt.bz2 On 2018-08-20 05:29, Alan Hayward wrote: > Use aapcs_is_vfp_call_or_return_candidate to detect float register > args, then pass in registers if there is room. > > pass_in_v_or_stack is no longer used. Remove it. Hi Alan, I didn't spot anything wrong, but then again I wouldn't trust myself to spot bugs in that code :). I noted some nits: > @@ -1522,19 +1522,57 @@ pass_in_x_or_stack (struct gdbarch *gdbarch, > struct regcache *regcache, > } > } > > -/* Pass a value in a V register, or on the stack if insufficient are > - available. */ > - > -static void > -pass_in_v_or_stack (struct gdbarch *gdbarch, > - struct regcache *regcache, > - struct aarch64_call_info *info, > - struct type *type, > - struct value *arg) > +/* Pass a value, which is of type arg_type, in a V register. Assumes > value is a > + aapcs_is_vfp_call_or_return_candidate and there are enough spare V > + registers. A return value of false is an error state as the value > will have > + been partially passed to the stack. */ > +static bool > +pass_in_v_vfp_candidate (struct gdbarch *gdbarch, struct regcache > *regcache, > + struct aarch64_call_info *info, struct type *arg_type, > + struct value *arg) > { > - if (!pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (type), > - value_contents (arg))) > - pass_on_stack (info, type, arg); > + switch (TYPE_CODE (arg_type)) > + { > + case TYPE_CODE_FLT: > + return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH > (arg_type), > + value_contents (arg)); > + break; > + > + case TYPE_CODE_COMPLEX: > + { > + const bfd_byte *buf = value_contents (arg); > + struct type *target_type = check_typedef (TYPE_TARGET_TYPE > (arg_type)); > + > + if (!pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (target_type), > + buf)) > + return false; > + > + return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (target_type), > + buf + TYPE_LENGTH (target_type)); > + } > + > + case TYPE_CODE_ARRAY: > + if (TYPE_VECTOR (arg_type)) > + return pass_in_v (gdbarch, regcache, info, TYPE_LENGTH (arg_type), > + value_contents (arg)); > + /* fall through. */ > + > + case TYPE_CODE_STRUCT: > + case TYPE_CODE_UNION: > + for (int i = 0; i < TYPE_NFIELDS (arg_type); i++) > + { > + struct value *field = value_primitive_field (arg, 0, i, > arg_type); > + struct type *field_type = check_typedef (value_type (field)); > + > + if (!pass_in_v_vfp_candidate (gdbarch, regcache, info, > field_type, > + field)) > + return false; > + } > + return true; I think the last line should have one fewer level of indentation. > /* Implement the "push_dummy_call" gdbarch method. */ > @@ -1623,12 +1661,33 @@ aarch64_push_dummy_call (struct gdbarch > *gdbarch, struct value *function, > for (argnum = 0; argnum < nargs; argnum++) > { > struct value *arg = args[argnum]; > - struct type *arg_type; > - int len; > + struct type *arg_type, *fundamental_type; > + int len, elements; > > arg_type = check_typedef (value_type (arg)); > len = TYPE_LENGTH (arg_type); > > + /* If arg can be passed in v registers as per the AAPCS64, then > do so if > + if there are enough spare registers. */ > + if (aapcs_is_vfp_call_or_return_candidate (arg_type, &elements, > + &fundamental_type)) > + { > + if (info.nsrn + elements <= 8) > + { > + /* We know that we have sufficient registers available > therefore > + this will never need to fallback to the stack. */ > + if (!pass_in_v_vfp_candidate (gdbarch, regcache, &info, > arg_type, > + arg)) > + error (_("Failed to push args")); I'm wondering if this could even be a gdb_assert, since it would be a logic error in GDB. We first checked that there are were enough spare registers to fit the argument. So if we fail to pass the argument in registers, it means our check was wrong, or something like that. In other words, this call failing can't result from bad input. Simon