D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 3632 - modify float is float to do a bitwise compare
Summary: modify float is float to do a bitwise compare
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P2 enhancement
Assignee: yebblies
URL:
Keywords: pull
: 8530 11442 (view as issue list)
Depends on:
Blocks:
 
Reported: 2009-12-18 12:04 UTC by Steven Schveighoffer
Modified: 2022-04-06 09:19 UTC (History)
9 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Steven Schveighoffer 2009-12-18 12:04:05 UTC
I was trying to test that my array append code is working properly.  In order to do this, I want to verify that when I set the length to a larger number that the additional space is filled with T.init

This test works great for most all types except floating point:

assert(a[$-1] is T.init)

It was pointed out that Phobos has isIdentical, but that doesn't work with literals for some reason:

float a;
assert(isIdentical(a, float.init)) // fails!

But it shouldn't be this hard.  Why does 'is' do a bitwise compare for everything *except* floating point numbers?  I understand that the spec states that is converts to == for builtin types, but it seems inconsistent because == for all builtin types except floating point *is* a bitwise compare.  It seems there is no easy way to get a bitwise compare for floating points, and the method provided doesn't work properly.  It should be braindead simple to get the compiler to do a bitwise compare.

I propose that for floating point types, x is y be equivalent to a bitwise compare.  It is easy to say in the spec "x is y does a bitwise compare, which for all builtin types except for floating point types is equivalent to equality"
Comment 1 Don 2009-12-18 22:49:41 UTC
(In reply to comment #0)
> It was pointed out that Phobos has isIdentical, but that doesn't work with
> literals for some reason:
> 
> float a;
> assert(isIdentical(a, float.init)) // fails!

That's because of the recently discovered NaN bug; the code generated for uninitialized floats and doubles needs to change. It's not a problem with isIdentical.

> But it shouldn't be this hard.  Why does 'is' do a bitwise compare for
> everything *except* floating point numbers?

> I propose that for floating point types, x is y be equivalent to a bitwise
> compare.  It is easy to say in the spec "x is y does a bitwise compare, which
> for all builtin types except for floating point types is equivalent to
> equality"

I think this would be a good idea. I've thought that several times myself. I created the isIdentical() function because I think there's a need for it.
Comment 4 bearophile_hugs 2011-06-26 00:49:51 UTC
Do you know what's missing in the list of bug 3981 ?
Comment 5 kennytm 2011-06-26 01:37:29 UTC
This is *not yet fixed*. The current implementation in DMD


           real_t v1 = e1->toReal();
           real_t v2 = e2->toReal();
           cmp = !memcmp(&v1, &v2, sizeof(real_t));


will not work, at least on OS X, because while 'real_t' ('long double') is only a 80-bit floating point number (occupying 10 bytes), with alignment 'sizeof(real_t)' will take 16 bytes. The extra 6 bytes of paddings are often filled with garbage. This makes even

       4.0 is 4.0

to return 'false'.
Comment 7 yebblies 2011-06-27 08:19:40 UTC
Reopening as the commit above will cause the following assert to fail:
static assert(real.init !is real.nan);

And the padding issue has only been fixed for the compile time evaluation, not runtime.
Comment 8 Walter Bright 2011-06-27 11:32:17 UTC
(In reply to comment #7)
> Reopening as the commit above will cause the following assert to fail:
> static assert(real.init !is real.nan);

This is intended. All nans are regarded as the same (even signalling and non-signalling).

> And the padding issue has only been fixed for the compile time evaluation, not
> runtime.

You're right.
Comment 9 bearophile_hugs 2011-06-27 13:33:12 UTC
(In reply to comment #8)

> This is intended. All nans are regarded as the same (even signalling and
> non-signalling).

So we have to use std.math.isIdentical() to tell apart floating point values on the base of the their bit patterns.

Related: I think almost no one uses the NaN payloads because (beside being a niche need) almost no language gives easy and explicit support to manage those payloads (while in std.math there are functions like getNaNPayload).
Comment 10 Walter Bright 2011-06-27 14:51:23 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > This is intended. All nans are regarded as the same (even signalling and
> > non-signalling).
> So we have to use std.math.isIdentical() to tell apart floating point values on
> the base of the their bit patterns.

Do you have any need for this?

> Related: I think almost no one uses the NaN payloads because (beside being a
> niche need) almost no language gives easy and explicit support to manage those
> payloads (while in std.math there are functions like getNaNPayload).

I've been around numerics for 35 years now, and I've never seen a use for NaN payloads. I've never seen anyone even propose a use. Until then, I suspect supporting such would just cause problems.
Comment 11 yebblies 2011-06-27 20:40:16 UTC
(In reply to comment #10)
> I've been around numerics for 35 years now, and I've never seen a use for NaN
> payloads. I've never seen anyone even propose a use. Until then, I suspect
> supporting such would just cause problems.

This report has always been asking for a bitwise comparion, which is (as far as I can tell) how every other type is treated by 'is'.

The use case I can remember being discussed is using 'v is float.init' to determine if a floating point value is uninitialized or is a nan due to the result of a calculation.

This seems to be the reason float.nan and float.init have different payloads in the first place.

Currently this fails:

struct A { float f; }
A a;
A b;
b.f = float.nan;
assert((a is b) is (a.f is b.f));

Making 'is' for floating point types consistent with 'is' for other types seems to me like a better move than introducing a new special case.

Maybe Steven or Don have an opinion on this feature they asked for? 'isIdentical' seems to do a straight bitwise comparison.
Comment 12 Walter Bright 2011-06-27 21:08:47 UTC
(In reply to comment #11)
> Making 'is' for floating point types consistent with 'is' for other types seems
> to me like a better move than introducing a new special case.

The current behavior matches template argument matching. Making it different from such will introduce all kinds of anomalous behavior.
Comment 13 yebblies 2011-06-27 21:25:22 UTC
(In reply to comment #12)
> The current behavior matches template argument matching. Making it different
> from such will introduce all kinds of anomalous behavior.

Fair enough.  I'm not sure which way is more useful, but they should definitely match.
Comment 14 Don 2011-06-28 01:08:25 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > This is intended. All nans are regarded as the same (even signalling and
> > > non-signalling).
> > So we have to use std.math.isIdentical() to tell apart floating point values on
> > the base of the their bit patterns.
> 
> Do you have any need for this?

I use it all the time, mainly for distinguishing between +0.0 and -0.0

But I think this has perhaps not been understood in this discussion: bitwise compare doesn't just affect NaN, it also means that +0.0 !is -0.0.

There definitely seems value in changing 'is' so that 'A is A' is true for any A. Note that any user-defined type can define == to always return false, so it isn't true that 'A is B' implies 'A==B', so we don't lose consistency in that way.

But it's reasonable to argue that '+0.0 is -0.0' should return true, and if you do that, then 'NaN is NaN' should also return true, regardless of the payload.

> > Related: I think almost no one uses the NaN payloads because (beside being a
> > niche need) almost no language gives easy and explicit support to manage those
> > payloads (while in std.math there are functions like getNaNPayload).
> 
> I've been around numerics for 35 years now, and I've never seen a use for NaN
> payloads. I've never seen anyone even propose a use. Until then, I suspect
> supporting such would just cause problems.

NaN payloads were used extensively in Apple's SANE, to distinguish different error types. Eg,

http://doc.4d.com/4D-Language-Reference-11.6/Error-Codes/SANE-NaN-Errors-1-255.300-206143.en.html
Comment 15 Walter Bright 2011-06-28 02:17:47 UTC
(In reply to comment #14)
> I use it all the time, mainly for distinguishing between +0.0 and -0.0

'is' does distinguish between +0 and -0. Just not -Nan and +Nan, nor the Nan payloads.

> But it's reasonable to argue that '+0.0 is -0.0' should return true, and if you
> do that, then 'NaN is NaN' should also return true, regardless of the payload.

I believe those are entirely different situations. The sign of 0 has a mathematical meaning to fp arithmetic, the sign/payload of Nan does not.

> NaN payloads were used extensively in Apple's SANE, to distinguish different
> error types. Eg,
> http://doc.4d.com/4D-Language-Reference-11.6/Error-Codes/SANE-NaN-Errors-1-255.300-206143.en.html

I didn't know that. But it's worth noting that SANE has been dropped.
Comment 16 Don 2011-06-28 04:43:29 UTC
(In reply to comment #15)
> (In reply to comment #14)
> > I use it all the time, mainly for distinguishing between +0.0 and -0.0
> 
> 'is' does distinguish between +0 and -0. Just not -Nan and +Nan, nor the Nan
> payloads.

Then I don't understand the reasoning. I don't think the payload NaN behaviour is terribly important, but being able to do bitwise compare gives an _enormous_ speed benefit.

Note that it isn't possible to create a NaN with a payload at compile time, so I don't think that the argument based on template behaviour is relevant -- as I see it, either behaviour is reasonable.

> > But it's reasonable to argue that '+0.0 is -0.0' should return true, and if you
> > do that, then 'NaN is NaN' should also return true, regardless of the payload.
> 
> I believe those are entirely different situations. The sign of 0 has a
> mathematical meaning to fp arithmetic, the sign/payload of Nan does not.

Yes, but my argument was the other way around: if you're going to special trouble with -0, you should deal with NaNs as well. But now, if you haven't given -0 special treatment, why slow down 'is' for the sake of the incredibly obscure NaN payload case?

Why not just say, "it does a bitwise compare, which is fast but may give unexpected answers in the case of negative zero and NaNs"?
Comment 17 Steven Schveighoffer 2011-06-28 04:56:43 UTC
(In reply to comment #11)
> The use case I can remember being discussed is using 'v is float.init' to
> determine if a floating point value is uninitialized or is a nan due to the
> result of a calculation.
> 
[snip]
> 
> Maybe Steven or Don have an opinion on this feature they asked for?
> 'isIdentical' seems to do a straight bitwise comparison.

Quite simply, the above (v is float.init) is my use case.  In some parts of the code, the runtime is responsible for default initializing data.  I wanted to verify that my code was properly initializing the data to T.init.

This is impossible without casting the value to a ubyte[] array to do a comparison (and even then, it's not quite right, because you can have garbage data in some cases).

But it makes no sense to me for is to ever do anything but a bitwise compare.

In other words, given any type T (and I mean any type T):

foo(T t)
{
assert(t is t);
}

should always pass.  It makes no sense to me for is to do anything else -- it's low-level bitwise comparison that bypasses any operators.

To make this true for every type *except* floating point types seems like a huge inconsistency.
Comment 18 Steven Schveighoffer 2011-06-28 05:02:11 UTC
(In reply to comment #7)
> Reopening as the commit above will cause the following assert to fail:
> static assert(real.init !is real.nan);

If they are not the same bit pattern, I think this is fine.  is should be a bitwise compare.  I don't know enough about floating point to know whether they are the same bit pattern.

In my understanding of floating point, this means that:

if(x is typeof(x).nan) ...

is not wise code -- it may fail if the exact nan bitpattern is different (my understanding is that nan has multiple representations).  While it may be unintuitive, that is no fault of D -- floating point is sometimes very unintuitive.
Comment 19 Steven Schveighoffer 2011-07-18 06:37:46 UTC
(In reply to comment #18)
> (In reply to comment #7)
> > Reopening as the commit above will cause the following assert to fail:
> > static assert(real.init !is real.nan);
> 
> If they are not the same bit pattern, I think this is fine.  is should be a
> bitwise compare.  I don't know enough about floating point to know whether they
> are the same bit pattern.

I think I misread the above.  If the above assert fails, and they are different bit patterns, this is definitely a problem.  Two different bit patterns should fail to compare as equal.

== can do the intelligent standards-conforming thing, but 'is' should do bitwise comparison.  If it doesn't, then you cannot use it as a tool to verify low-level manipulation (such as allocating memory with a pre-defined value) is working.  And I don't see the point of using 'is' on floating points otherwise, it's misleading.
Comment 20 yebblies 2012-02-15 22:07:55 UTC
New pull for this issue:

https://github.com/D-Programming-Language/dmd/pull/724

Walter, please reconsider your position - I really believe that the advantages of 'is' always being a straight bitwise comparison outweigh the downsides.  As you've pointed the inconsistency between how floats are handled with 'is' and when determining if template value parameters match, I've changed that too to make it consistent.
Comment 21 bearophile_hugs 2012-02-16 04:44:31 UTC
(In reply to comment #20)

> - I really believe that the advantages
> of 'is' always being a straight bitwise comparison outweigh the downsides.

This is what I was asking in the closed down issue 3981
Comment 22 yebblies 2012-02-16 05:08:26 UTC
(In reply to comment #21)
> This is what I was asking in the closed down issue 3981

I know, that's why I closed issue 3981 with a comment saying that part was covered by this report...
Comment 24 Walter Bright 2012-02-17 17:22:43 UTC
change reverted - does not work
Comment 26 github-bugzilla 2012-04-27 15:06:20 UTC
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/f9c21c2a77e32d310c8cb9d6c23f517d410cbec6
Reintroduce the fix for issue 3632, without the dependency on fixing issue 1824.

Revert the parts in clone.c that were commented out, so pull #387 can merge cleanly.

https://github.com/D-Programming-Language/dmd/commit/5da38bd430246d82e06d5581bb15325094821ca5
Merge pull request #730 from yebblies/issue3632

Reintroduce the fix for issue 3632, without the dependency on fixing issue 1824.
Comment 27 Walter Bright 2012-04-27 15:43:54 UTC
Reverted because their tests failed.
Comment 29 Denis Shelomovskii 2013-11-05 01:53:25 UTC
So currently we have this:
---
void main()
{
    float f1, f2 = float.nan;
    assert(f1 !is f2); // ok

    import std.math;
    assert(NaN(0) !is NaN(1)); // ok
    assert(NaN(1) !is NaN(2)); // ok
}

static assert({
    float f1, f2 = float.nan;
    assert(f1 !is f2); // error, the only failing test
    return 1;
}());
---
Comment 30 Denis Shelomovskii 2013-11-05 02:02:09 UTC
Looks like we have bitwise comparison at runtime, but `f_is` generates some long assembly code in contrast to `i_is`:
---
bool f_is(float f1, float f2)
{ return f1 is f2; }

bool i_is(int i1, int i2)
{ return i1 is i2; }
---

Can somebody explain what currently does `float is float` do in runtime?
Comment 31 yebblies 2013-11-05 02:20:15 UTC
(In reply to comment #30)
> Looks like we have bitwise comparison at runtime, but `f_is` generates some
> long assembly code in contrast to `i_is`:
> ---
> bool f_is(float f1, float f2)
> { return f1 is f2; }
> 
> bool i_is(int i1, int i2)
> { return i1 is i2; }
> ---
> 
> Can somebody explain what currently does `float is float` do in runtime?

Calls memcmp ( https://github.com/D-Programming-Language/dmd/pull/724/files#diff-6e3ab8a500e476994f345ede433811bbR2479 )
Comment 32 yebblies 2013-11-14 23:02:51 UTC
*** Issue 11442 has been marked as a duplicate of this issue. ***
Comment 33 Denis Shelomovskii 2015-01-14 12:01:14 UTC
(In reply to yebblies from comment #32)
> *** Issue 11442 has been marked as a duplicate of this issue. ***

Testcase from Issue 11442:
---
struct S { float n; }
static assert(S.init is S.init); // fails
---
Comment 34 Daniel Kozak 2019-05-28 19:58:26 UTC
*** Issue 8530 has been marked as a duplicate of this issue. ***
Comment 35 Dlang Bot 2019-05-30 07:19:15 UTC
@ibuclaw updated dlang/dmd pull request #7568 "fix Issue 3632 - modify float is float to do a bitwise compare" fixing this issue:

- fix Issue 3632 - modify float is float to do a bitwise compare

https://github.com/dlang/dmd/pull/7568
Comment 37 Nathan S. 2020-06-07 03:36:27 UTC
This was incorrectly marked fixed. `float is float` is still not a bitwise comparison.

---
import std.math : isIdentical;
assert(!isIdentical(float.nan, -float.nan)); // passes
assert(float.nan is -float.nan); // also passes
---
Comment 38 Dlang Bot 2022-03-08 10:45:27 UTC
@dkorpel created dlang/dmd pull request #13780 "fix Issue 3632 - modify float is float to do a bitwise compare" fixing this issue:

- fix Issue 3632 - modify float is float to do a bitwise compare
  
  update TOK & target
  
  fix xtest46
  
  Change back to original ufcs test
  
  Update changes

https://github.com/dlang/dmd/pull/13780
Comment 39 Dlang Bot 2022-04-06 09:19:54 UTC
dlang/dmd pull request #13780 "fix Issue 3632 - modify float is float to do a bitwise compare" was merged into master:

- 6974a1e2185e29603329860ecfc85822bcfc9657 by Dennis Korpel:
  fix Issue 3632 - modify float is float to do a bitwise compare

https://github.com/dlang/dmd/pull/13780