public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Joseph Myers <joseph@codesourcery.com>
To: Terry Guo <terry.xpguo@gmail.com>
Cc: "H.J. Lu" <hjl.tools@gmail.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	Uros Bizjak <ubizjak@gmail.com>
Subject: Re: [PATCH] x86-64: Use TI->SF and TI->DF conversions in soft-fp
Date: Tue, 22 Jan 2019 00:58:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.21.1901220042220.27116@digraph.polyomino.org.uk> (raw)
In-Reply-To: <CAATtE7CUE4ZwF54=O7uCkoT7iCKkfYcs5dtBZXUExG1KG4Y3Dg@mail.gmail.com>

On Tue, 22 Jan 2019, Terry Guo wrote:

> Hi Joseph,
> 
> I believe HJ is proposing patch to fix bug
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88931. In the test case
> of the bug, the "#pragma STDC FENV_ACCESS ON" is used and there are

Which isn't supported by GCC.  Any test involving rounding modes should 
ensure inputs and results are volatile (or use asm, etc., but volatile is 
simpler for tests) to make sure that computations aren't moved across 
rounding mode changes (which can happen even with -frounding-math, 
-frounding-math only affects a few things like constant folding, without 
preventing such code movement).

> The current _floattisf from libgcc2 doesn't support those four rounding modes.

It doesn't need to mention rounding modes anywhere.  The basic design of 
all those conversion functions is that, given the input, they determine 
other inputs to other conversions with the property that only a single 
floating-point rounding occurs in the sequence of operations and that the 
input to that rounding is similar enough to the input to the original 
operation (through careful use of sticky bits etc.) that the result of 
that rounding will always be the correct result of the original operation, 
independent of the rounding mode.

For example, it's always valid, in any rounding mode, to convert TImode to 
SFmode by changing the TImode input to a nicer one (at most top 64 bits 
nonzero, say, so that conversions from DImode can be used as an 
intermediate step) such that the top 25 bits (starting with the first 
nonzero bit, for positive or unsigned arguments) of the two values agree, 
and the two values also agree in whether any lower-order bits are nonzero.  
That sort of thing is what the code in libgcc2.c is trying to do.

Some of that logic is complex, and it's entirely possible it has bugs.  
But the correct fix must be an architecture-independent one in libgcc2.c; 
any architecture-specific version is just a subsequent optimization on top 
of that.  In general, for any bug, you need to work out where the buggy 
code is (meaning understanding the intended interfaces between bits of the 
compiler that are involved in this question), and fix it there rather than 
doing a target-specific workaround.  If you want to do a target-specific 
workaround (like this patch is), you need to call out up front that your 
patch is just a workaround and give strong justification for that approach 
(e.g. some way in which the proper general fix would be destabilizing at 
the current development stage).

The current description of the bug "Wrong __floattisf and __floattidf are 
selected in libgcc" is completely inappropriate unless the assertion is 
that one of the #if conditionals in libgcc2.c is wrong (in which case that 
#if conditional, or the code it guards, should be corrected).

-- 
Joseph S. Myers
joseph@codesourcery.com

  reply	other threads:[~2019-01-22  0:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-21 16:16 H.J. Lu
2019-01-21 16:43 ` Uros Bizjak
2019-01-21 16:56   ` H.J. Lu
2019-01-21 16:59     ` Uros Bizjak
2019-01-21 17:10       ` H.J. Lu
2019-01-21 18:37         ` H.J. Lu
2019-01-21 23:48 ` Joseph Myers
2019-01-22  0:40   ` Terry Guo
2019-01-22  0:58     ` Joseph Myers [this message]
2019-01-22  1:34       ` H.J. Lu
2019-01-22  2:03         ` Joseph Myers
2019-01-23  8:19           ` H.J. Lu

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=alpine.DEB.2.21.1901220042220.27116@digraph.polyomino.org.uk \
    --to=joseph@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hjl.tools@gmail.com \
    --cc=terry.xpguo@gmail.com \
    --cc=ubizjak@gmail.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).