refactor Add()
I decided to separate out the refactoring of https://github.com/dlang/dmd/pull/14694 into this PR to make 14694 easier to deal with.
Thanks for your pull request, @WalterBright!
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + dmd#14695"
It's a lot more readable than the previous version. It took me a while to figure out what was going on in that, this one is much easier to read and verify correctness.
This is about CTFE and has nothing to do with code gen. The code gen was fixed a while ago to round to float, the CTFE is a bit behind.
There shouldn't be separation between C11 and D, or at least it should be minimized.
This PR does not change the semantics at all. What it provides is an easy platform to focus on how to make addf and addd round the way we need it to without concern for the rest of the code.
I know it doesn't properly handle excess precision. This is a refactoring, not a fix. It produces identical results.
I don't know how the actual fix could be anything other than a rewrite of addf and addd. Which is most of the point of this refactoring - the problem gets isolated to two functions that are easy to understand.
I know that the front end never stores floats in any format other than real_t. This is not a problem since floats and doubles are exact subsets of real_t.
This does not appear that it will scale well to the other operations
One thing at a time. I don't like making gigantic PRs. This is a nice, self-contained PR, easy to review.
@ibuclaw you requested changes, but I don't know what those are.
It's a lot more readable than the previous version. It took me a while to figure out what was going on in that, this one is much easier to read and verify correctness.
This is about CTFE and has nothing to do with code gen. The code gen was fixed a while ago to round to float, the CTFE is a bit behind.
I can see that, but I also notice x86-32 produces wrong code too - this would be related to the loss of excess precision (32-bit evaluates to precision of 80-bit real except on OSX).
There shouldn't be separation between C11 and D, or at least it should be minimized.
The spec couldn't be any more clear that it's different.
C11
Floating constants are evaluated to a format whose range and precision may be greater than required by the type. The use of evaluation formats is characterized by the implementation-defined value of #
FLT_EVAL_METHOD.
D2
Regardless of the type of the operands, floating-point constant folding is done in real or greater precision.
Something needs to change, either keep with the spec, or change the spec - changing the spec would be a huge departure from your rejection of CTFE rounding at Dconf 2012/13 when Don demanded it.
Changing the behaviour now needs a clear deprecation path.
I know it doesn't properly handle excess precision. This is a refactoring, not a fix. It produces identical results.
I don't know how the actual fix could be anything other than a rewrite of addf and addd. Which is most of the point of this refactoring - the problem gets isolated to two functions that are easy to understand.
I would imagine:
- Query target for the excess precision type of left and right hand side expression.
- Round intermediate values to that type if less than current precision.
- Perform operation.
- Round the result (though maybe we can get away without doing this).
At no point do I forsee addf/addd being needed during any of those steps. I am more than willing to be proven wrong! :-)
This PR addresses none of those issues because it is a refactoring, with zero change in behavior.
at no point do I forsee addf/addd being needed
The rounding issue is 100% how addf and addd behave. It has nothing to do with the semantics of complex numbers. What happens with compile time (float + float), rounding and all, then becomes 100% how addf is implemented.
This PR addresses none of those issues because it is a refactoring, with zero change in behavior.
at no point do I forsee addf/addd being needed
The rounding issue is 100% how addf and addd behave. It has nothing to do with the semantics of complex numbers. What happens with compile time (float + float), rounding and all, then becomes 100% how addf is implemented.
No, it's 100% how floating point is implemented, as e1.toReal() + e2.toReal() is always done at highest precision at compile-time (192-bits in gdc) regardless of what type the expression is fixed to (x + y -> x.opBinary!"+"(y)), so the general approach has to be on what rounding occurs around the operation, which doesn't care about whether it's an add/sub/mul/div/pow.
Focusing on the individual addf/addd looks to be a great way to get x86-centric results (and only 64-bit/SSE, not 32-bit/387!), but that ignores every other kind of system out there that might have different precision requirements for float op float or double op double.
it's 100% how floating point is implemented, as e1.toReal() + e2.toReal() is always done at highest precision at compile-time
Yes, because this is a refactoring and that is how it is done before and after this PR.
Focusing on the individual addf/addd looks to be a great way to get x86-centric results (and only 64-bit/SSE, not 32-bit/387!), but that ignores every other kind of system out there that might have different precision requirements for float op float or double op double.
This is a refactoring to put the dependency on floating point rounding into one function. It does not change how dmd rounds. Changing how dmd rounds add for float is then encapsulated in one function rather that being spread around the compiler.