lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] MinGW-w64 anomaly?


From: Greg Chicares
Subject: Re: [lmi] MinGW-w64 anomaly?
Date: Tue, 27 Dec 2016 17:22:17 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0

On 2016-12-26 17:37, Vadim Zeitlin wrote:
> On Mon, 26 Dec 2016 08:37:22 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> For the time being, does the following patch work on x86_64, and if so,
> GC> should I apply it?
> 
>  This patch does try to do the right thing, but fails because of 2 things:
> first, it doesn't compile due to duplicate e_ieee754_rounding declaration
> and, second, just removing as advised by the comment isn't enough because
> we still pass the wrong values to fesetround() (which took me some time to
> find out -- isn't it great to have untyped interfaces in the C++ standard
> library...).

Yick. In 'fenv_lmi_x86.hpp', I had written:

/// Rounding-control values. The enumerators are lowercase versions of
/// the C99 7.6/7 macros, although the constant-expressions may differ.

enum e_ieee754_rounding
    {fe_tonearest  = 0x00
    ,fe_downward   = 0x01
    ,fe_upward     = 0x02
    ,fe_towardzero = 0x03
    };

because C99 [7.6/7] says FE_TONEAREST etc. are "defined if and only if
the implementation supports getting and setting the represented rounding
direction by means of the fegetround and fesetround functions", and at
least when that was written, we were supporting implementations that
didn't support those things. Therefore, I had to set my own values, and
I chose to copy intel's because that's the most reasonable thing to do
for an intel architecture, and it would match any implementation that
also does the most reasonable thing. But I guess gcc did something
different, perhaps for good reasons that I just don't understand.

Now, however, we're using C++11, which says [26.3.1/1] that <cfenv>
"defines the macros", seemingly without qualification, except that
[26.3.1/2] it "the same as Clause 7.6 of the C standard", which seems
to bring in the C99 "if and only if" qualification (depending on the
meaning of the word "same").

Of course, I'd prefer to define
 enum e_ieee754_rounding
-    {fe_tonearest  = 0x00
+    {fe_tonearest  = FE_TONEAREST
and so on, but I guess that's not guaranteed to work, whereas the way
you wrote it is reliably correct.

>  Of course, this is not the final version if only because it doesn't make
> much sense to reproduce the code which is already present (but inside
> LMI_IEC_559 check) in fend_rounding() itself, so I'd rather change it
> instead.

I think I should apply your patch as soon as I have a chance to test it,
because we shouldn't live with preventable unit-test failures--even though
you plan to replace it soon with something better. I'd like to change some
other rounding stuff, and it's a bad idea to do that while unit tests are
broken.

>  But I'm still not quite sure whether I should work on the patch doing
> what, I think, we've now agreed to do or if I should leave it to you?

Let me commit the patch you shared, and then leave the rest to you.

> Again, my idea is to reuse the existing LMI_IEC_559 code and always enable
> it for platforms not using x87, i.e. i386 code if the symbol indicating
> the use of SSE is not defined, and all the other platforms.

I guess that sounds right. The questions I have are:

- For x86_64, does gcc allow no other option than SSE? (I gather that
it still uses the x87 e.g. for trigonometric functions, but doesn't
offer an option to use the x87 for everything.)

- Whenever we set the SSE rounding direction, should we also set the
x87 rounding direction compatibly? (If we didn't, then trigonometric
calculations might be rounded in the wrong direction AIUI.)

>  Should I do it, test it and submit it as a proper patch/pull request?

Yes, please.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]