Skip to content

Reimplement php_round_helper() using modf() #12220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ DOM:

Standard:
. Implement GH-12188 (Indication for the int size in phpinfo()). (timwolla)
. Partly fix GH-12143 (Incorrect round() result for 0.49999999999999994).
(timwolla)

<<< NOTE: Insert NEWS from last stable release here prior to actual release! >>>
9 changes: 9 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ PHP 8.4 UPGRADE NOTES
5. Changed Functions
========================================

- Standard:
. The internal implementation for rounding to integers has been rewritten
to be easier to verify for correctness and to be easier to maintain.
Some rounding bugs have been fixed as a result of the rewrite. For
example previously rounding 0.49999999999999994 to the nearest integer
would have resulted in 1.0 instead of the correct result 0.0. Additional
inputs might also be affected and result in different outputs compared to
earlier PHP versions.

========================================
6. New Functions
========================================
Expand Down
82 changes: 64 additions & 18 deletions ext/standard/math.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,27 +93,73 @@ static inline double php_intpow10(int power) {
/* {{{ php_round_helper
Actually performs the rounding of a value to integer in a certain mode */
static inline double php_round_helper(double value, int mode) {
double tmp_value;
double integral, fractional;

/* Split the input value into the integral and fractional part.
*
* Both parts will have the same sign as the input value. We take
* the absolute value of the fractional part (which will not result
* in branches in the assembly) to make the following cases simpler.
*/
fractional = fabs(modf(value, &integral));

switch (mode) {
case PHP_ROUND_HALF_UP:
if (fractional >= 0.5) {
/* We must increase the magnitude of the integral part
* (rounding up / towards infinity). copysign(1.0, integral)
* will either result in 1.0 or -1.0 depending on the sign
* of the input, thus increasing the magnitude, but without
* generating branches in the assembly.
*
* This pattern is equally used for all the other modes.
*/
return integral + copysign(1.0, integral);
}

if (value >= 0.0) {
tmp_value = floor(value + 0.5);
if ((mode == PHP_ROUND_HALF_DOWN && value == (-0.5 + tmp_value)) ||
(mode == PHP_ROUND_HALF_EVEN && value == (0.5 + 2 * floor(tmp_value/2.0))) ||
(mode == PHP_ROUND_HALF_ODD && value == (0.5 + 2 * floor(tmp_value/2.0) - 1.0)))
{
tmp_value = tmp_value - 1.0;
}
} else {
tmp_value = ceil(value - 0.5);
if ((mode == PHP_ROUND_HALF_DOWN && value == (0.5 + tmp_value)) ||
(mode == PHP_ROUND_HALF_EVEN && value == (-0.5 + 2 * ceil(tmp_value/2.0))) ||
(mode == PHP_ROUND_HALF_ODD && value == (-0.5 + 2 * ceil(tmp_value/2.0) + 1.0)))
{
tmp_value = tmp_value + 1.0;
}
return integral;

case PHP_ROUND_HALF_DOWN:
if (fractional > 0.5) {
return integral + copysign(1.0, integral);
}

return integral;

case PHP_ROUND_HALF_EVEN:
if (fractional > 0.5) {
return integral + copysign(1.0, integral);
}

if (fractional == 0.5) {
bool even = !fmod(integral, 2.0);

/* If the integral part is not even we can make it even
* by adding one in the direction of the existing sign.
*/
if (!even) {
return integral + copysign(1.0, integral);
}
}

return integral;
case PHP_ROUND_HALF_ODD:
if (fractional > 0.5) {
return integral + copysign(1.0, integral);
}

if (fractional == 0.5) {
bool even = !fmod(integral, 2.0);

if (even) {
return integral + copysign(1.0, integral);
}
}

return integral;
}

return tmp_value;
ZEND_UNREACHABLE();
}
/* }}} */

Expand Down
27 changes: 27 additions & 0 deletions ext/standard/tests/math/round_gh12143_1.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-12143: Test rounding of 0.49999999999999994.
--FILE--
<?php
foreach ([
0.49999999999999994,
-0.49999999999999994,
] as $number) {
foreach ([
'PHP_ROUND_HALF_UP',
'PHP_ROUND_HALF_DOWN',
'PHP_ROUND_HALF_EVEN',
'PHP_ROUND_HALF_ODD',
] as $mode) {
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
}
}
?>
--EXPECT--
PHP_ROUND_HALF_UP : +0.49999999999999994 -> +0
PHP_ROUND_HALF_DOWN : +0.49999999999999994 -> +0
PHP_ROUND_HALF_EVEN : +0.49999999999999994 -> +0
PHP_ROUND_HALF_ODD : +0.49999999999999994 -> +0
PHP_ROUND_HALF_UP : -0.49999999999999994 -> -0
PHP_ROUND_HALF_DOWN : -0.49999999999999994 -> -0
PHP_ROUND_HALF_EVEN : -0.49999999999999994 -> -0
PHP_ROUND_HALF_ODD : -0.49999999999999994 -> -0
27 changes: 27 additions & 0 deletions ext/standard/tests/math/round_gh12143_2.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-12143: Test rounding of 0.50000000000000011.
--FILE--
<?php
foreach ([
0.50000000000000011,
-0.50000000000000011,
] as $number) {
foreach ([
'PHP_ROUND_HALF_UP',
'PHP_ROUND_HALF_DOWN',
'PHP_ROUND_HALF_EVEN',
'PHP_ROUND_HALF_ODD',
] as $mode) {
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
}
}
?>
--EXPECT--
PHP_ROUND_HALF_UP : +0.50000000000000011 -> +1
PHP_ROUND_HALF_DOWN : +0.50000000000000011 -> +1
PHP_ROUND_HALF_EVEN : +0.50000000000000011 -> +1
PHP_ROUND_HALF_ODD : +0.50000000000000011 -> +1
PHP_ROUND_HALF_UP : -0.50000000000000011 -> -1
PHP_ROUND_HALF_DOWN : -0.50000000000000011 -> -1
PHP_ROUND_HALF_EVEN : -0.50000000000000011 -> -1
PHP_ROUND_HALF_ODD : -0.50000000000000011 -> -1
27 changes: 27 additions & 0 deletions ext/standard/tests/math/round_gh12143_3.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-12143: Test rounding of 0.5.
--FILE--
<?php
foreach ([
0.5,
-0.5,
] as $number) {
foreach ([
'PHP_ROUND_HALF_UP',
'PHP_ROUND_HALF_DOWN',
'PHP_ROUND_HALF_EVEN',
'PHP_ROUND_HALF_ODD',
] as $mode) {
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
}
}
?>
--EXPECT--
PHP_ROUND_HALF_UP : +0.5 -> +1
PHP_ROUND_HALF_DOWN : +0.5 -> +0
PHP_ROUND_HALF_EVEN : +0.5 -> +0
PHP_ROUND_HALF_ODD : +0.5 -> +1
PHP_ROUND_HALF_UP : -0.5 -> -1
PHP_ROUND_HALF_DOWN : -0.5 -> -0
PHP_ROUND_HALF_EVEN : -0.5 -> -0
PHP_ROUND_HALF_ODD : -0.5 -> -1
27 changes: 27 additions & 0 deletions ext/standard/tests/math/round_gh12143_4.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-12143: Test rounding of 1.5.
--FILE--
<?php
foreach ([
1.5,
-1.5,
] as $number) {
foreach ([
'PHP_ROUND_HALF_UP',
'PHP_ROUND_HALF_DOWN',
'PHP_ROUND_HALF_EVEN',
'PHP_ROUND_HALF_ODD',
] as $mode) {
printf("%-20s: %+.17g -> %+.17g\n", $mode, $number, round($number, 0, constant($mode)));
}
}
?>
--EXPECT--
PHP_ROUND_HALF_UP : +1.5 -> +2
PHP_ROUND_HALF_DOWN : +1.5 -> +1
PHP_ROUND_HALF_EVEN : +1.5 -> +2
PHP_ROUND_HALF_ODD : +1.5 -> +1
PHP_ROUND_HALF_UP : -1.5 -> -2
PHP_ROUND_HALF_DOWN : -1.5 -> -1
PHP_ROUND_HALF_EVEN : -1.5 -> -2
PHP_ROUND_HALF_ODD : -1.5 -> -1