From: Jim Wilson <jimw@sifive.com>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes.
Date: Tue, 13 Nov 2018 01:52:00 -0000 [thread overview]
Message-ID: <CAFyWVaYmm6gXVOk5EcsFrqKkd2Nudfsc9q+kmYd0F07ztUkUtA@mail.gmail.com> (raw)
In-Reply-To: <20181108154344.GA16539@embecosm.com>
On Thu, Nov 8, 2018 at 7:43 AM Andrew Burgess
<andrew.burgess@embecosm.com> 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
prev parent reply other threads:[~2018-11-13 1:52 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-06 21:43 [PATCH 0/3] " Jim Wilson
2018-11-06 21:44 ` [PATCH 3/3] " Jim Wilson
2018-11-08 14:34 ` Andrew Burgess
2018-11-08 14:44 ` Andrew Burgess
2018-11-06 21:44 ` [PATCH 2/3] " Jim Wilson
2018-11-08 14:37 ` Andrew Burgess
2018-11-06 21:44 ` [PATCH 1/3] " Jim Wilson
2018-11-08 15:43 ` Andrew Burgess
2018-11-13 1:52 ` Jim Wilson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFyWVaYmm6gXVOk5EcsFrqKkd2Nudfsc9q+kmYd0F07ztUkUtA@mail.gmail.com \
--to=jimw@sifive.com \
--cc=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox