From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 87170 invoked by alias); 13 Nov 2018 01:52:52 -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 87138 invoked by uid 89); 13 Nov 2018 01:52:51 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=funny X-HELO: mail-vs1-f66.google.com Received: from mail-vs1-f66.google.com (HELO mail-vs1-f66.google.com) (209.85.217.66) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Nov 2018 01:52:50 +0000 Received: by mail-vs1-f66.google.com with SMTP id h18so6332880vsj.4 for ; Mon, 12 Nov 2018 17:52:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=kJpqHgjcBkoH/f/11WAVz4aJR4oQZ6L0vFBRWW4E4v8=; b=JekbgLqs5jLiwVw3qPvHeXcN+7OiJEOwirp6fWd+XacFk0eEDjZEftRWEdBQafn6iR 1Uz2HzxmEqu2cVLm7YFyj2PAVLq8+9V0yS/YLMfBJ+S4fDg10gdL9ZJnSN+c3/LmvhRp IyoLNAfjZkegxGxmxU2kLFUSuAZpYz13Dmq8SLnH982170kfovRYC6c+Pfn3VLx7uB3C inA7vSTmSec3egNAHZJAbE40J55dhrUkJvtxgULGO10L9hVEY5Fe9Gh82tdvUWSiexmK g5wuSU0ZOWrUXQrlPXQt1P5gwBPrU82Ky7qAr3A84U6giQxEyfxey3XnBgMeQqfBS1E3 IlnQ== MIME-Version: 1.0 References: <20181106214403.22192-1-jimw@sifive.com> <20181108154344.GA16539@embecosm.com> In-Reply-To: <20181108154344.GA16539@embecosm.com> From: Jim Wilson Date: Tue, 13 Nov 2018 01:52:00 -0000 Message-ID: Subject: Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes. To: Andrew Burgess Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-11/txt/msg00214.txt.bz2 On Thu, Nov 8, 2018 at 7:43 AM Andrew Burgess wrote: > I struggled to understand what the difference between align and > slot_align is in this patch, it feels like.... If you have an argument twice the size of xlen, with alignment twice the size of xlen, then when we pass the first word, align is 2*xlen and slot_align is 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. > - 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 you do this, and we keep the patch in riscv_assign_stack_location, then we end up allocating 2*xlen bytes for the first xlen bytes of a argument of size 2*len whcih is wrong. I suppose you want to drop the riscv_assign_stack_location change too. That could work, but that leaves arg_offset unaligned after allocating the last argument. Though it looks like the only problem with that is some funny riscv_debug_infcall output. It might say for instance that we have 12 bytes of stack arguments for a 64-bit target, which doesn't make any sense. For a 64-bit target, stack arguments should always be a multiple of 8 bytes. Otherwise, this looks harmless, and this is a problem that we already have, so this isn't anything broken by this patch set. I'll rewrite the patch this way. Jim