From patchwork Thu Nov 8 15:43:44 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 30077 Received: (qmail 5475 invoked by alias); 8 Nov 2018 15:43:51 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 5463 invoked by uid 89); 8 Nov 2018 15:43:51 -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= X-HELO: mail-wr1-f67.google.com Received: from mail-wr1-f67.google.com (HELO mail-wr1-f67.google.com) (209.85.221.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 08 Nov 2018 15:43:49 +0000 Received: by mail-wr1-f67.google.com with SMTP id j26-v6so21778981wre.1 for ; Thu, 08 Nov 2018 07:43:49 -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=24IjEZG2wkMdTBeqgq9CCtVf+yAKX4qZT1TJVD7zJFs=; b=CktWqnWeNaW83m4dVX5KnDFUDEFOkhZrOfHQFfbM/j6A2nrQNuE7bZFu/gh6bIk/RG Kameb0QlQfDctrEM1Q+0wImfBNN3QAfTbGcivRxv9iYieq7+ZZvC75MC9MdpqlN0wH9N xL5RUZgaNHgGyLxhJRMgYr+z9tsX70Lq+OcgwBFXFfihRza90J12xzbbT6BXn2EDxKPK OGoPfd8a3gvOFMT7OgjM7AS8/wzT5+SBAudZgN5CeJogeszrhmfLJrzezQv+d2h7zhB6 CSziA4Rgipm0HmZ/coA2djvaa/c5asd6CTywZNRlruYA/yzbh2xW/48Tb8r/5wXxqM6r whrQ== Return-Path: Received: from localhost ([2a02:390:741d:1:3665:d267:b319:d766]) by smtp.gmail.com with ESMTPSA id z7-v6sm3834549wrv.21.2018.11.08.07.43.46 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 08 Nov 2018 07:43:46 -0800 (PST) Date: Thu, 8 Nov 2018 15:43:44 +0000 From: Andrew Burgess To: Jim Wilson Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes. Message-ID: <20181108154344.GA16539@embecosm.com> References: <20181106214403.22192-1-jimw@sifive.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20181106214403.22192-1-jimw@sifive.com> X-Fortune: Take it easy, we're in a hurry. X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Jim Wilson [2018-11-06 13:44:03 -0800]: > Ensure that stack slots are always the same size as XLEN by rounding up arg > sizes when computing the address of the next stack slot. > > gdb/ > * riscv-tdep.c (riscv_assign_stack_location): New arg slot_align. > Use with align_up when setting arg_offset. > (riscv_call_arg_scalar_int): Call riscv_assign_stack_location with > cinfo->xlen as new arg. > --- > gdb/riscv-tdep.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c > index db372e2163..ac4f2533f4 100644 > --- a/gdb/riscv-tdep.c > +++ b/gdb/riscv-tdep.c > @@ -1868,13 +1868,13 @@ riscv_assign_reg_location (struct riscv_arg_info::location *loc, > static void > riscv_assign_stack_location (struct riscv_arg_info::location *loc, > struct riscv_memory_offsets *memory, > - int length, int align) > + int length, int align, int slot_align) I struggled to understand what the difference between align and slot_align is in this patch, it feels like.... > { > loc->loc_type = riscv_arg_info::location::on_stack; > memory->arg_offset > = align_up (memory->arg_offset, align); > loc->loc_data.offset = memory->arg_offset; > - memory->arg_offset += length; > + memory->arg_offset += align_up (length, slot_align); > loc->c_length = length; > > /* Offset is always 0, either we're the first location part, in which > @@ -1919,7 +1919,7 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > cinfo->xlen, 0)) > riscv_assign_stack_location (&ainfo->argloc[1], > &cinfo->memory, cinfo->xlen, > - cinfo->xlen); > + cinfo->xlen, cinfo->xlen); > } > else > { > @@ -1928,7 +1928,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > if (!riscv_assign_reg_location (&ainfo->argloc[0], > &cinfo->int_regs, len, 0)) > riscv_assign_stack_location (&ainfo->argloc[0], > - &cinfo->memory, len, ainfo->align); > + &cinfo->memory, len, ainfo->align, > + cinfo->xlen); .... you might be better off just passing a better value of align through here. Testing on gdb.base/gnu_vector.exp shows that doing this fixes the same problem that your original patch fixes. My revision is below, but I've only just kicked off a test run, so there might be problems I'm not seeing. Thanks, Andrew > > if (len < ainfo->length) > { > @@ -1937,7 +1938,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, > &cinfo->int_regs, len, > cinfo->xlen)) > riscv_assign_stack_location (&ainfo->argloc[1], > - &cinfo->memory, len, cinfo->xlen); > + &cinfo->memory, len, cinfo->xlen, > + cinfo->xlen); > } > } > } > -- > 2.17.1 > diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c index a0b2d1f5d7..243db12999 100644 --- a/gdb/riscv-tdep.c +++ b/gdb/riscv-tdep.c @@ -1951,12 +1951,13 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo, } else { - int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length; + int len = std::min (ainfo->length, cinfo->xlen); + int align = std::max (ainfo->align, cinfo->xlen); if (!riscv_assign_reg_location (&ainfo->argloc[0], &cinfo->int_regs, len, 0)) riscv_assign_stack_location (&ainfo->argloc[0], - &cinfo->memory, len, ainfo->align); + &cinfo->memory, len, align); if (len < ainfo->length) {