Skip to content

Fix bug 9937 CTFE floats don't overflow correctly#5191

Closed
ibuclaw wants to merge 1 commit into
dlang:masterfrom
ibuclaw:issue9937
Closed

Fix bug 9937 CTFE floats don't overflow correctly#5191
ibuclaw wants to merge 1 commit into
dlang:masterfrom
ibuclaw:issue9937

Conversation

@ibuclaw
Copy link
Copy Markdown
Member

@ibuclaw ibuclaw commented Oct 11, 2015

Revamp of #3590

@WalterBright
Copy link
Copy Markdown
Member

https://issues.dlang.org/show_bug.cgi?id=9937 comment 4.

This was hashed out there. I do not agree that it Is a bug - it is working as designed, and there are good reasons for that design. I don't believe that 'fixing' it fixes anything - the problems just shift elsewhere.

At some point, if you work with floating point code, you're simply going to have to understand how it works rather than assume it behaves like mathematics does.

In particular, be very careful when using == with floating point operands.

@ibuclaw
Copy link
Copy Markdown
Member Author

ibuclaw commented Oct 12, 2015

I'm certainly not against keeping the excess precision for the lifetime of the compiler. The backend will always discard excess precision when emitting the data in the end.

I'm just not sure about your assertion that CTFE operations should fail by design when the opposite happens at runtime. I could argue that we already do this kind of discarding with integers (IntegerExp.normalize), and as far as I can tell it's for the same reason.

The only alternative to this that would be my contribution to the table would be to add two methods to RealExp, toDouble and toFloat (or asDouble) which returns a copy of the real that has been truncated to fit the lower precision requested.

This, for the purpose of when we want to read the value at it's declared precision at key points, such as in assignments and comparisons. For all other operations, the original is used intact.

All our data types up to 64 bits are precisely defined, so I don't think it is any stretch to ensure that we give at least some consistent behaviour in both runtime and CTFE.

@yebblies
Copy link
Copy Markdown
Contributor

I'm just not sure about your assertion that CTFE operations should fail by design when the opposite happens at runtime.

The problem is that runtime behavior is NOT guaranteed. Exact precision will depend on inlining, register allocation and instruction selection. It makes it very difficult to match the runtime behavior exactly...

@ibuclaw
Copy link
Copy Markdown
Member Author

ibuclaw commented Oct 13, 2015

The problem is that runtime behavior is NOT guaranteed.

If it wasn't guaranteed, then we wouldn't have extensive unittests in std.math and gammafunction. If dmd is not IEEE 754 compliant with its codegen by default, then that would be a bug.

You should not be using x87 registers for doubles and floats, so inlining and register allocation should not be an issue.

@yebblies
Copy link
Copy Markdown
Contributor

How exactly does this violate IEEE754?

The same thing can occur without x87, eg

a * b + c == a * b + c

Where the LHS is evaluated using multiply+add with a high internal precision while the RHS uses discrete multiply and add.

@WalterBright
Copy link
Copy Markdown
Member

I agree with adding functions toFloat and toDouble for those rare cases where precision is intended to be discarded. But in general, more precision == better. Intentionally desiring less precise results in order to satisfy an incorrect notion of equality is indicative of faulty algorithm design rather than a compiler bug.

@ibuclaw
Copy link
Copy Markdown
Member Author

ibuclaw commented Oct 16, 2015

Intentionally desiring less precise results in order to satisfy an incorrect notion of equality

When comparing two floats, you only ever ask that the relevant bits are equal (eg: memcmp is used for reals). So it seems correct to me that if you have const float == const float where both sides have been CTFE'd to a value, you wouldn't compare them as const double == const real or as any other mismatch combination.

@WalterBright
Copy link
Copy Markdown
Member

floats are used to 1) increase speed 2) reduce memory consumption. They are not used to generate less precise results outside of very rare cases, and test suites.

Besides, if this is pulled, the problems will just show up elsewhere. There is no fixing it. There is no intuitive answer. The rules set up cater to the most common, by far, usage scenario, which is "gimme the most precise answer practical." float/double/real specify the minimum precision, not the maximum.

@9il
Copy link
Copy Markdown
Member

9il commented Apr 15, 2016

I agree with adding functions toFloat and toDouble for those rare cases where precision is intended to be discarded. But in general, more precision == better.

More precision is not better. In many math functions (see CEPHES) IEEE are assumed and compensator add/sub are used to improve result. More precision may break computation result. This is common case in math.

@WalterBright
Copy link
Copy Markdown
Member

More precision is not better. In many math functions (see CEPHES) IEEE are assumed and compensator add/sub are used to improve result. More precision may break computation result. This is common case in math.

For an algorithm that depends on reduced precision, it should use toFloat or toDouble. Doing this not only guarantees it is performed, it implicitly documents the requirement. This PR is the wrong solution.

@9il
Copy link
Copy Markdown
Member

9il commented Apr 17, 2016

For an algorithm that depends on reduced precision, it should use toFloat or toDouble. Doing this not only guarantees it is performed, it implicitly documents the requirement. This PR is the wrong solution.

And how I can implement this functions to be portable and CTFEable?

@yebblies
Copy link
Copy Markdown
Contributor

And how I can implement this functions to be portable and CTFEable?

As intrinsics.

@9il
Copy link
Copy Markdown
Member

9il commented Apr 17, 2016

As intrinsics.

They would be really helpfull. For example I would be able to clean mir from workarounds and warnings

@ibuclaw ibuclaw deleted the issue9937 branch April 17, 2016 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants