public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Burgess <andrew.burgess@embecosm.com>
To: Jim Wilson <jimw@sifive.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 2/2] gcc/riscv: Add a mechanism to remove some calls to _riscv_save_0
Date: Mon, 28 Oct 2019 17:18:00 -0000	[thread overview]
Message-ID: <20191028163007.GN4962@embecosm.com> (raw)
In-Reply-To: <CAFyWVaY3UmhED0_zk42ayXxageYPJB9Sr1d_h-tUDBuK2e5Uqw@mail.gmail.com>

* Jim Wilson <jimw@sifive.com> [2019-10-22 16:34:53 -0700]:

> On Mon, Oct 21, 2019 at 5:26 AM Andrew Burgess
> <andrew.burgess@embecosm.com> wrote:
> > Below is a new versions of this patch, I believe that this addresses
> > the review comments from the earlier version.  In addition this has
> > been tested using Jim's idea of forcing -msave-restore (if the flag is
> > not otherwise given) and I now see no test failures for newlib and
> > linux toolchains.
> >
> > One thing that has been mentioned briefly was adding a flag to control
> > just this optimisation, I haven't done that yet, but can if that is
> > required - obviously this optimisation doesn't do anything if
> > -msave-restore is turned off, so that is always an option.  With no
> > test failures I don't know if a separate flag is required.
> 
> I can live without the flag to control it, since as you mention
> -msave-restore will turn it off.
> 
> I'm seeing failures with the new save-restore-4.c testcase for 32-bit
> targets.  It works for 64-bit but not for 32-bit, because the
> sign-extension it is checking for only happens for 64-bit targets.
> The testcase should check for a target of riscv64*-*-* or else hard
> code -march=rv64gc etc options.  Either fix seems reasonable.
> 
> You fixed some British spellings of optimise to optimize, but there
> are two new comments that add two more uses of optimise which is
> inconsistent.
> 
> I also noticed a "useage" that should be "usage".
> 
> Otherwise this looks good to me.  It is OK to check in with the above
> minor issues fixed.

Jim,

Thanks for all your help reviewing this patch.

I've now merged this with fixes for the issues you identified above.
Let me know if you run into any problems.

Thanks,
Andrew

  reply	other threads:[~2019-10-28 16:30 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 19:31 [PATCH 0/2] RISCV: Reduce code size when compiling with -msave-restore Andrew Burgess
2019-08-19 19:47 ` [PATCH 2/2] gcc/riscv: Add a mechanism to remove some calls to _riscv_save_0 Andrew Burgess
2019-08-23  7:46   ` Jim Wilson
2019-08-26  2:17     ` Jim Wilson
2019-08-31  2:55       ` Jim Wilson
2019-08-31 13:12         ` Andreas Schwab
2019-08-31 13:57         ` Andreas Schwab
2019-09-06 23:41           ` Jim Wilson
2019-08-24 17:18   ` Jim Wilson
2019-10-21 12:53   ` Andrew Burgess
2019-10-23  0:59     ` Jim Wilson
2019-10-28 17:18       ` Andrew Burgess [this message]
2019-08-19 19:50 ` [PATCH 1/2] gcc/riscv: Include more registers in SIBCALL_REGS Andrew Burgess
2019-08-19 19:54   ` Andrew Waterman
2019-08-23  7:13   ` Jim Wilson
2019-08-24 10:55     ` Jim Wilson
2019-10-16 21:14   ` [PATCH] RISC-V: " Jim Wilson
2019-10-17 14:15     ` Andrew Burgess
2019-10-17 22:05       ` Jim Wilson
2019-10-17 22:22         ` Andrew Burgess

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=20191028163007.GN4962@embecosm.com \
    --to=andrew.burgess@embecosm.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jimw@sifive.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).