From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 129694 invoked by alias); 8 Nov 2018 14:34:19 -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 129629 invoked by uid 89); 8 Nov 2018 14:34:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=row, HX-Received:sk:b11-v6m, arg_type, sk:riscv_c X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 08 Nov 2018 14:34:16 +0000 Received: by mail-wr1-f65.google.com with SMTP id z16-v6so21503045wrv.2 for ; Thu, 08 Nov 2018 06:34:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=7Ep05ZIk+qvjq+f5yOinNOEjMbanbtlAo4Nd/RVxm6I=; b=fWcn4twl5eR4w8taDYM+Q+oz7h1q7/obCv44ClL7EyUNS2YDYIy9+70LEc6ZNuMI6P ggrDr1hHRIdBXsa9c1hOs9FMhbMnabo40DYdc9bC5Bm0H29f0q9YnJECspl3+7hNAv7P 7k2LNkWO4PzqUCcZxHyhuYAIOQSpxmxNbocrnp+Ep86ZfU15ZsVvaN1ChiJaNQUTmuzv cIJmq9/6v5oF43VEak3ZCylGjgLGt0v/QAnSVMxqxzXK7EDoV/LlKQdmgARUfdk/+Reh GtEnybagEKT2dInszlUxuqA7OiLRZWNnOaoHbxpmKRcO51UImcZa/RNI3/5nPSEOYF0k IY+w== Return-Path: Received: from localhost ([2a02:390:741d:1:3665:d267:b319:d766]) by smtp.gmail.com with ESMTPSA id 21-v6sm5737351wmv.5.2018.11.08.06.34.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 08 Nov 2018 06:34:12 -0800 (PST) Date: Thu, 08 Nov 2018 14:34:00 -0000 From: Andrew Burgess To: Jim Wilson Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 3/3] RISC-V: gdb.base/gnu_vector fixes. Message-ID: <20181108143411.GX16539@embecosm.com> References: <20181106214446.22325-1-jimw@sifive.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181106214446.22325-1-jimw@sifive.com> X-Fortune: The second best policy is dishonesty. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00117.txt.bz2 * Jim Wilson [2018-11-06 13:44:46 -0800]: > Unnamed arguments with 2*XLEN alignment are passed in aligned register pairs. > > gdb/ > * riscv-tdep.c (struct riscv_arg_info): New field is_unnamed. > (riscv_call_arg_scalar_int): If unnamed arg with twice xlen alignment, > then increment next_regnum if odd. > (riscv_arg_location): New arg is_unnamed. Set ainfo->is_unnamed. > (riscv_push_dummy_call): New local ftype. Call check_typedef to set > function type. Pass new arg to riscv_arg_location based on function > type. > (riscv_return_value): Pass new arg to riscv_arg_location. Could you change the titles for the 3 commits in this series please to make them more descriptive of the fix in each commit. This also avoids having 3 commits in a row with the same title line, which can look like a mistake in 'git log --format=oneline'. > --- > gdb/riscv-tdep.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index 3d4f7e3dcc..93310c329f 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -1737,6 +1737,9 @@ struct riscv_arg_info > then this offset will be set to 0. */ > int c_offset; > } argloc[2]; > + > + /* TRUE if this is an unnamed argument. */ > + bool is_unnamed; > }; > > /* Information about a set of registers being used for passing arguments as > @@ -1932,6 +1935,12 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > { > int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; > > + /* Unnamed arguments in registers that require 2*XLEN alignment are > + passed in an aligned register pair. */ > + if (ainfo->is_unnamed && (ainfo->align == cinfo->xlen * 2) > + && cinfo->int_regs.next_regnum & 1) > + cinfo->int_regs.next_regnum++; > + > if (!riscv_assign_reg_location (&ainfo->argloc[0], > &cinfo->int_regs, len, 0)) > riscv_assign_stack_location (&ainfo->argloc[0], > @@ -2222,11 +2231,12 @@ static void > riscv_arg_location (struct gdbarch *gdbarch, > struct riscv_arg_info *ainfo, > struct riscv_call_info *cinfo, > - struct type *type) > + struct type *type, bool is_unnamed) Could you update the functions header comment to explain what impact IS_UNNAMED will have on the behaviour of the function. > { > ainfo->type = type; > ainfo->length = TYPE_LENGTH (ainfo->type); > ainfo->align = riscv_type_alignment (ainfo->type); > + ainfo->is_unnamed = is_unnamed; > ainfo->contents = nullptr; > > switch (TYPE_CODE (ainfo->type)) > @@ -2375,6 +2385,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > > CORE_ADDR osp = sp; > > + struct type *ftype = check_typedef (value_type (function)); > + > + if (TYPE_CODE (ftype) == TYPE_CODE_PTR) > + ftype = check_typedef (TYPE_TARGET_TYPE (ftype)); > + > /* We'll use register $a0 if we're returning a struct. */ > if (struct_return) > ++call_info.int_regs.next_regnum; > @@ -2388,7 +2403,8 @@ riscv_push_dummy_call (struct gdbarch *gdbarch, > arg_value = args[i]; > arg_type = check_typedef (value_type (arg_value)); > > - riscv_arg_location (gdbarch, info, &call_info, arg_type); > + riscv_arg_location (gdbarch, info, &call_info, arg_type, > + TYPE_VARARGS (ftype) && i >= TYPE_NFIELDS (ftype)); > > if (info->type != arg_type) > arg_value = value_cast (info->type, arg_value); > @@ -2565,7 +2581,7 @@ riscv_return_value (struct gdbarch *gdbarch, > struct type *arg_type; > > arg_type = check_typedef (type); > - riscv_arg_location (gdbarch, &info, &call_info, arg_type); > + riscv_arg_location (gdbarch, &info, &call_info, arg_type, false); > > if (riscv_debug_infcall > 0) > { > -- > 2.17.1 > Otherwise looks fine. Thanks, Andrew