public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Georg Johann Lay <avr@gjlay.de>, Jonathan Wakely <jwakely@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch, avr] Fix PR target/99184: Wrong cast from double to 16-bit and 32-bit ints.
Date: Mon, 19 Sep 2022 11:06:26 +0200	[thread overview]
Message-ID: <CAFiYyc1GXVAYn-_KQJgjFpiXNoVrcq-871W1Rd2V_S3x2gSd+A@mail.gmail.com> (raw)
In-Reply-To: <8f612783-6bfb-30a8-b755-447664a5272d@gjlay.de>

On Mon, Sep 19, 2022 at 10:57 AM Georg Johann Lay <avr@gjlay.de> wrote:
>
>
>
> Am 19.09.22 um 09:51 schrieb Richard Biener:
> > On Sun, Sep 18, 2022 at 7:40 PM Georg Johann Lay <avr@gjlay.de> wrote:
> >>
> >> Hello,
> >>
> >> this patch fixed PR target/99184 which incorrectly rounded during 64-bit
> >> (long) double to 16-bit and 32-bit integers.
> >>
> >> The patch just removes the respective roundings from
> >> libf7-asm.sx::to_integer and ::to_unsigned.  Luckily, LibF7 does nowhere
> >> use respective functions internally, the only user is in libf7.c::f7_exp
> >>
> >> which reads
> >>
> >>     f7_round (qq, qq);
> >>     int16_t q = f7_get_s16 (qq);
> >>
> >> so that f7_get_s16() operates on an already rounded value, and therefore
> >> this code works unaltered with or without rounding in to_integer.
> >>
> >> The patch applies to directory
> >>
> >> ./libgcc/config/avr/libf7/
> >>
> >> and is the same for all GCC versions v10+.
> >>
> >> Please someone with write permissions commit it to trunk and backport to
> >> v12, v11, and v10 as it is a wrong-code issue.
> >>
> >> The patch will fit without problems (except for ChangeLog) because there
> >> is no traffic on that folder.
> >
> > Thanks, I've pushed the change.  Please in future try to send patches
> > that can be applied with git am, thus use git format-patch
> >
> > Richard.
>
> Thanks you so much.  The patch I generated with "git diff > file.diff",
> so that is not correct?

Yes, it's correct, but see below.

> The only change is that I defined extra hunks
> for asm so that one can see the function like in
>
>   @@ -601,9 +601,6 @@ DEFUN to_integer
>
> So git is not prepared to such hunks? Would you point me to some
> documentation on how to do it properly?

The more useful process is that after changing the code you
do a git commit to your tree and then

git format-patch --stdout HEAD^

to get a git patch.  On that I can do 'git am' and that will produce
exactly the commit you produced, including the patch description
and ChangeLog parts.  I had to add that manually.

Indeed https://gcc.gnu.org/contribute.html doesn't mention this,
we should probably improve that.  Jonathan, maybe the above
is not the optimal way so can you think of something to add
to the 'Submitting Patches' section to document this?

Thanks,
Richard.

>
> Thanks,
>
> Johann

  reply	other threads:[~2022-09-19  9:06 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-18 17:39 [patch,avr] " Georg Johann Lay
2022-09-19  7:51 ` [patch, avr] " Richard Biener
2022-09-19  8:57   ` Georg Johann Lay
2022-09-19  9:06     ` Richard Biener [this message]
2022-09-19 14:58       ` Jonathan Wakely

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=CAFiYyc1GXVAYn-_KQJgjFpiXNoVrcq-871W1Rd2V_S3x2gSd+A@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=avr@gjlay.de \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jwakely@redhat.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).