From 4ea2ccd6fcb256cf2985af1bdf0e5ef56334dbdd Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 6 May 2023 11:18:03 +0100 Subject: [PATCH 1/8] ext/standard/array.c: Optimize min/max functions for int/float --- ext/standard/array.c | 100 ++++++++++++++++-- .../array/max_int_float_optimisation.phpt | 69 ++++++++++++ .../array/min_int_float_optimisation.phpt | 69 ++++++++++++ 3 files changed, 230 insertions(+), 8 deletions(-) create mode 100644 ext/standard/tests/array/max_int_float_optimisation.phpt create mode 100644 ext/standard/tests/array/min_int_float_optimisation.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 46c2c882b83d6..2d94405c2d834 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1237,11 +1237,53 @@ PHP_FUNCTION(min) uint32_t i; min = &args[0]; + zend_long min_lval; + double min_dval; - for (i = 1; i < argc; i++) { - is_smaller_function(&result, &args[i], min); - if (Z_TYPE(result) == IS_TRUE) { - min = &args[i]; + if (Z_TYPE_P(min) == IS_LONG) { + min_lval = Z_LVAL_P(min); + + for (i = 1; i < argc; i++) { + if (EXPECTED(Z_TYPE(args[i]) == IS_LONG)) { + if (min_lval > Z_LVAL(args[i])) { + min_lval = Z_LVAL(args[i]); + min = &args[i]; + } + } else if (Z_TYPE(args[i]) == IS_DOUBLE) { + min_dval = (double) min_lval; + goto double_compare; + } else { + goto generic_compare; + } + } + + RETURN_LONG(min_lval); + } else if (Z_TYPE_P(min) == IS_DOUBLE) { + min_dval = Z_DVAL_P(min); + + for (i = 1; i < argc; i++) { + if (EXPECTED(Z_TYPE(args[i]) == IS_DOUBLE)) { + double_compare: + if (min_dval > Z_DVAL(args[i])) { + min_dval = Z_DVAL(args[i]); + min = &args[i]; + } + } else if (Z_TYPE(args[i]) == IS_LONG) { + if (min_dval > (double)Z_LVAL(args[i])) { + min_dval = (double)Z_LVAL(args[i]); + min = &args[i]; + } + } else { + goto generic_compare; + } + } + } else { + for (i = 1; i < argc; i++) { + generic_compare: + is_smaller_function(&result, &args[i], min); + if (Z_TYPE(result) == IS_TRUE) { + min = &args[i]; + } } } @@ -1283,11 +1325,53 @@ PHP_FUNCTION(max) uint32_t i; max = &args[0]; + zend_long max_lval; + double max_dval; - for (i = 1; i < argc; i++) { - is_smaller_or_equal_function(&result, &args[i], max); - if (Z_TYPE(result) == IS_FALSE) { - max = &args[i]; + if (Z_TYPE_P(max) == IS_LONG) { + max_lval = Z_LVAL_P(max); + + for (i = 1; i < argc; i++) { + if (EXPECTED(Z_TYPE(args[i]) == IS_LONG)) { + if (max_lval < Z_LVAL(args[i])) { + max_lval = Z_LVAL(args[i]); + max = &args[i]; + } + } else if (Z_TYPE(args[i]) == IS_DOUBLE) { + max_dval = (double) max_lval; + goto double_compare; + } else { + goto generic_compare; + } + } + + RETURN_LONG(max_lval); + } else if (Z_TYPE_P(max) == IS_DOUBLE) { + max_dval = Z_DVAL_P(max); + + for (i = 1; i < argc; i++) { + if (EXPECTED(Z_TYPE(args[i]) == IS_DOUBLE)) { + double_compare: + if (max_dval < Z_DVAL(args[i])) { + max_dval = Z_DVAL(args[i]); + max = &args[i]; + } + } else if (Z_TYPE(args[i]) == IS_LONG) { + if (max_dval < (double)Z_LVAL(args[i])) { + max_dval = (double)Z_LVAL(args[i]); + max = &args[i]; + } + } else { + goto generic_compare; + } + } + } else { + for (i = 1; i < argc; i++) { + generic_compare: + is_smaller_or_equal_function(&result, &args[i], max); + if (Z_TYPE(result) == IS_FALSE) { + max = &args[i]; + } } } diff --git a/ext/standard/tests/array/max_int_float_optimisation.phpt b/ext/standard/tests/array/max_int_float_optimisation.phpt new file mode 100644 index 0000000000000..e7c31b9c740e6 --- /dev/null +++ b/ext/standard/tests/array/max_int_float_optimisation.phpt @@ -0,0 +1,69 @@ +--TEST-- +Check max() optimisation for int and float types +--FILE-- + +--EXPECT-- +Start as int optimisation: +int(10) +int(10) +int(10) +int(10) +int(10) +int(10) +string(2) "15" +Start as float optimisation: +float(10.5) +float(10.5) +float(10.5) +float(10.5) +float(10.5) +float(10.5) +string(4) "15.5" diff --git a/ext/standard/tests/array/min_int_float_optimisation.phpt b/ext/standard/tests/array/min_int_float_optimisation.phpt new file mode 100644 index 0000000000000..9d80dec5f811e --- /dev/null +++ b/ext/standard/tests/array/min_int_float_optimisation.phpt @@ -0,0 +1,69 @@ +--TEST-- +Check min() optimisation for int and float types +--FILE-- + +--EXPECT-- +Start as int optimisation: +int(2) +int(2) +int(2) +int(2) +int(2) +int(2) +string(1) "1" +Start as float optimisation: +float(2.5) +float(2.5) +float(2.5) +float(2.5) +float(2.5) +float(2.5) +string(3) "1.5" From a08f5a34408222b29880b10c5efd58546506a572 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 6 May 2023 13:42:35 +0100 Subject: [PATCH 2/8] Use zend_compare Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com> --- ext/standard/array.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 2d94405c2d834..942f87e3a0172 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1280,7 +1280,9 @@ PHP_FUNCTION(min) } else { for (i = 1; i < argc; i++) { generic_compare: - is_smaller_function(&result, &args[i], min); + if (zend_compare(&args[i], min) < 0) { + min = &args[i]; + } if (Z_TYPE(result) == IS_TRUE) { min = &args[i]; } From fb609dc887ceaaa30764a43ba6f7dee00c98fcaa Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 6 May 2023 13:45:16 +0100 Subject: [PATCH 3/8] Cleanup + max() --- ext/standard/array.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 942f87e3a0172..951fb59046343 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1233,7 +1233,7 @@ PHP_FUNCTION(min) } } else { /* mixed min ( mixed $value1 , mixed $value2 [, mixed $value3... ] ) */ - zval *min, result; + zval *min; uint32_t i; min = &args[0]; @@ -1283,9 +1283,6 @@ PHP_FUNCTION(min) if (zend_compare(&args[i], min) < 0) { min = &args[i]; } - if (Z_TYPE(result) == IS_TRUE) { - min = &args[i]; - } } } @@ -1323,7 +1320,7 @@ PHP_FUNCTION(max) } } else { /* mixed max ( mixed $value1 , mixed $value2 [, mixed $value3... ] ) */ - zval *max, result; + zval *max; uint32_t i; max = &args[0]; @@ -1370,8 +1367,7 @@ PHP_FUNCTION(max) } else { for (i = 1; i < argc; i++) { generic_compare: - is_smaller_or_equal_function(&result, &args[i], max); - if (Z_TYPE(result) == IS_FALSE) { + if (zend_compare(&args[i], max) > 0) { max = &args[i]; } } From b7a11f8e29565d2f785614363d3dc28743f4bfa8 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sun, 7 May 2023 15:16:18 +0100 Subject: [PATCH 4/8] Fix test + int losing precision with float fix --- ext/standard/array.c | 8 +-- .../array/max_int_float_optimisation.phpt | 70 +++++++++++-------- .../array/min_int_float_optimisation.phpt | 70 +++++++++++-------- 3 files changed, 88 insertions(+), 60 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 951fb59046343..14ded6375e907 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1249,7 +1249,7 @@ PHP_FUNCTION(min) min_lval = Z_LVAL(args[i]); min = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_DOUBLE) { + } else if (Z_TYPE(args[i]) == IS_DOUBLE && ((zend_long)(double) min_lval == min_lval)) { min_dval = (double) min_lval; goto double_compare; } else { @@ -1268,7 +1268,7 @@ PHP_FUNCTION(min) min_dval = Z_DVAL(args[i]); min = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_LONG) { + } else if (Z_TYPE(args[i]) == IS_LONG && ((zend_long)(double) Z_LVAL(args[i]) == Z_LVAL(args[i]))) { if (min_dval > (double)Z_LVAL(args[i])) { min_dval = (double)Z_LVAL(args[i]); min = &args[i]; @@ -1336,7 +1336,7 @@ PHP_FUNCTION(max) max_lval = Z_LVAL(args[i]); max = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_DOUBLE) { + } else if (Z_TYPE(args[i]) == IS_DOUBLE && ((zend_long)(double) max_lval == max_lval)) { max_dval = (double) max_lval; goto double_compare; } else { @@ -1355,7 +1355,7 @@ PHP_FUNCTION(max) max_dval = Z_DVAL(args[i]); max = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_LONG) { + } else if (Z_TYPE(args[i]) == IS_LONG && ((zend_long)(double) Z_LVAL(args[i]) == Z_LVAL(args[i]))) { if (max_dval < (double)Z_LVAL(args[i])) { max_dval = (double)Z_LVAL(args[i]); max = &args[i]; diff --git a/ext/standard/tests/array/max_int_float_optimisation.phpt b/ext/standard/tests/array/max_int_float_optimisation.phpt index e7c31b9c740e6..10646246b326b 100644 --- a/ext/standard/tests/array/max_int_float_optimisation.phpt +++ b/ext/standard/tests/array/max_int_float_optimisation.phpt @@ -1,53 +1,63 @@ --TEST-- Check max() optimisation for int and float types +--SKIPIF-- + --FILE-- --EXPECT-- @@ -59,6 +69,8 @@ int(10) int(10) int(10) string(2) "15" +Check that int not representable as float works: +int(-9223372036854775807) Start as float optimisation: float(10.5) float(10.5) @@ -67,3 +79,5 @@ float(10.5) float(10.5) float(10.5) string(4) "15.5" +Check that int not representable as float works: +int(-9223372036854775807) diff --git a/ext/standard/tests/array/min_int_float_optimisation.phpt b/ext/standard/tests/array/min_int_float_optimisation.phpt index 9d80dec5f811e..c43b9a5e2bf34 100644 --- a/ext/standard/tests/array/min_int_float_optimisation.phpt +++ b/ext/standard/tests/array/min_int_float_optimisation.phpt @@ -1,53 +1,63 @@ --TEST-- Check min() optimisation for int and float types +--SKIPIF-- + --FILE-- --EXPECT-- @@ -59,6 +69,8 @@ int(2) int(2) int(2) string(1) "1" +Check that int not representable as float works: +int(9223372036854775806) Start as float optimisation: float(2.5) float(2.5) @@ -67,3 +79,5 @@ float(2.5) float(2.5) float(2.5) string(3) "1.5" +Check that int not representable as float works: +int(9223372036854775806) From 1dff2be98f239f1023035f4255843dec3c5ce501 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 8 May 2023 10:05:00 +0100 Subject: [PATCH 5/8] Elide UB --- ext/standard/array.c | 8 ++++---- .../array/max_int_float_optimisation.phpt | 18 ++++++++++++++++++ .../array/min_int_float_optimisation.phpt | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 14ded6375e907..c6db6f9281b91 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1249,7 +1249,7 @@ PHP_FUNCTION(min) min_lval = Z_LVAL(args[i]); min = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_DOUBLE && ((zend_long)(double) min_lval == min_lval)) { + } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) min_lval) == min_lval)) { min_dval = (double) min_lval; goto double_compare; } else { @@ -1268,7 +1268,7 @@ PHP_FUNCTION(min) min_dval = Z_DVAL(args[i]); min = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_LONG && ((zend_long)(double) Z_LVAL(args[i]) == Z_LVAL(args[i]))) { + } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { if (min_dval > (double)Z_LVAL(args[i])) { min_dval = (double)Z_LVAL(args[i]); min = &args[i]; @@ -1336,7 +1336,7 @@ PHP_FUNCTION(max) max_lval = Z_LVAL(args[i]); max = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_DOUBLE && ((zend_long)(double) max_lval == max_lval)) { + } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) max_lval) == max_lval)) { max_dval = (double) max_lval; goto double_compare; } else { @@ -1355,7 +1355,7 @@ PHP_FUNCTION(max) max_dval = Z_DVAL(args[i]); max = &args[i]; } - } else if (Z_TYPE(args[i]) == IS_LONG && ((zend_long)(double) Z_LVAL(args[i]) == Z_LVAL(args[i]))) { + } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { if (max_dval < (double)Z_LVAL(args[i])) { max_dval = (double)Z_LVAL(args[i]); max = &args[i]; diff --git a/ext/standard/tests/array/max_int_float_optimisation.phpt b/ext/standard/tests/array/max_int_float_optimisation.phpt index 10646246b326b..01483131f11f5 100644 --- a/ext/standard/tests/array/max_int_float_optimisation.phpt +++ b/ext/standard/tests/array/max_int_float_optimisation.phpt @@ -31,6 +31,13 @@ echo "Check that int not representable as float works:\n"; var_dump(max( PHP_INT_MIN+1, PHP_INT_MIN, PHP_INT_MIN*2) ); +var_dump(max( + PHP_INT_MAX-1, PHP_INT_MAX, PHP_INT_MAX*2) +); +// Has INF +var_dump(max( + PHP_INT_MAX-1, PHP_INT_MAX, PHP_INT_MAX**20) +); echo "Start as float optimisation:\n"; var_dump(max( @@ -58,6 +65,13 @@ echo "Check that int not representable as float works:\n"; var_dump(max( PHP_INT_MIN*2, PHP_INT_MIN, PHP_INT_MIN+1) ); +var_dump(max( + PHP_INT_MAX*2, PHP_INT_MAX, PHP_INT_MAX-1) +); +// Has INF +var_dump(max( + PHP_INT_MAX**20, PHP_INT_MAX, PHP_INT_MAX-1) +); ?> --EXPECT-- @@ -71,6 +85,8 @@ int(10) string(2) "15" Check that int not representable as float works: int(-9223372036854775807) +float(1.8446744073709552E+19) +float(INF) Start as float optimisation: float(10.5) float(10.5) @@ -81,3 +97,5 @@ float(10.5) string(4) "15.5" Check that int not representable as float works: int(-9223372036854775807) +float(1.8446744073709552E+19) +float(INF) diff --git a/ext/standard/tests/array/min_int_float_optimisation.phpt b/ext/standard/tests/array/min_int_float_optimisation.phpt index c43b9a5e2bf34..064fbddb5e3c9 100644 --- a/ext/standard/tests/array/min_int_float_optimisation.phpt +++ b/ext/standard/tests/array/min_int_float_optimisation.phpt @@ -31,6 +31,13 @@ echo "Check that int not representable as float works:\n"; var_dump(min( PHP_INT_MAX-1, PHP_INT_MAX, PHP_INT_MAX*2) ); +var_dump(min( + PHP_INT_MIN+1, PHP_INT_MIN, PHP_INT_MIN*2) +); +// Has INF +var_dump(min( + PHP_INT_MAX-1, PHP_INT_MAX, PHP_INT_MAX**20) +); echo "Start as float optimisation:\n"; var_dump(min( @@ -58,6 +65,13 @@ echo "Check that int not representable as float works:\n"; var_dump(min( PHP_INT_MAX*2, PHP_INT_MAX, PHP_INT_MAX-1) ); +var_dump(min( + PHP_INT_MIN*2, PHP_INT_MIN, PHP_INT_MIN+1) +); +// Has INF +var_dump(min( + PHP_INT_MAX**20, PHP_INT_MAX, PHP_INT_MAX-1) +); ?> --EXPECT-- @@ -71,6 +85,8 @@ int(2) string(1) "1" Check that int not representable as float works: int(9223372036854775806) +float(-1.8446744073709552E+19) +int(9223372036854775806) Start as float optimisation: float(2.5) float(2.5) @@ -81,3 +97,5 @@ float(2.5) string(3) "1.5" Check that int not representable as float works: int(9223372036854775806) +float(-1.8446744073709552E+19) +int(9223372036854775806) From 6edcc6b39d85c9470cd568961d713ae0b6d8bd7d Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 31 May 2023 11:27:37 +0100 Subject: [PATCH 6/8] Reformat tests --- .../array/max_int_float_optimisation.phpt | 80 +++++-------------- .../array/min_int_float_optimisation.phpt | 80 +++++-------------- 2 files changed, 40 insertions(+), 120 deletions(-) diff --git a/ext/standard/tests/array/max_int_float_optimisation.phpt b/ext/standard/tests/array/max_int_float_optimisation.phpt index 01483131f11f5..0f5df35d12a7c 100644 --- a/ext/standard/tests/array/max_int_float_optimisation.phpt +++ b/ext/standard/tests/array/max_int_float_optimisation.phpt @@ -6,72 +6,32 @@ Check max() optimisation for int and float types --EXPECT-- diff --git a/ext/standard/tests/array/min_int_float_optimisation.phpt b/ext/standard/tests/array/min_int_float_optimisation.phpt index 064fbddb5e3c9..e383b833694c7 100644 --- a/ext/standard/tests/array/min_int_float_optimisation.phpt +++ b/ext/standard/tests/array/min_int_float_optimisation.phpt @@ -6,72 +6,32 @@ Check min() optimisation for int and float types --EXPECT-- From 2a8940ddd542e4fca13bff25916eb074ad33ef33 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Wed, 31 May 2023 11:43:11 +0100 Subject: [PATCH 7/8] [skip ci] add comments --- ext/standard/array.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/standard/array.c b/ext/standard/array.c index c6db6f9281b91..47f19ec6ac4cd 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1250,6 +1250,7 @@ PHP_FUNCTION(min) min = &args[i]; } } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) min_lval) == min_lval)) { + /* if min_lval can be exactly represented as a float, go to float dedicated code */ min_dval = (double) min_lval; goto double_compare; } else { @@ -1269,6 +1270,7 @@ PHP_FUNCTION(min) min = &args[i]; } } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { + /* if the value can be exactly represented as a float, use float dedicated code otherwise generic */ if (min_dval > (double)Z_LVAL(args[i])) { min_dval = (double)Z_LVAL(args[i]); min = &args[i]; @@ -1337,6 +1339,7 @@ PHP_FUNCTION(max) max = &args[i]; } } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) max_lval) == max_lval)) { + /* if max_level can be exactly represented as a float, go to float dedicated code */ max_dval = (double) max_lval; goto double_compare; } else { @@ -1356,6 +1359,7 @@ PHP_FUNCTION(max) max = &args[i]; } } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { + /* if the value can be exactly represented as a float, use float dedicated code otherwise generic */ if (max_dval < (double)Z_LVAL(args[i])) { max_dval = (double)Z_LVAL(args[i]); max = &args[i]; From af45e84fe520540e615ef12deacde307f346fe56 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 2 Jun 2023 10:26:39 +0100 Subject: [PATCH 8/8] [skip ci] Comments fix --- ext/standard/array.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 47f19ec6ac4cd..6bb146eb46888 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -1250,7 +1250,7 @@ PHP_FUNCTION(min) min = &args[i]; } } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) min_lval) == min_lval)) { - /* if min_lval can be exactly represented as a float, go to float dedicated code */ + /* if min_lval can be exactly represented as a double, go to double dedicated code */ min_dval = (double) min_lval; goto double_compare; } else { @@ -1270,7 +1270,7 @@ PHP_FUNCTION(min) min = &args[i]; } } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { - /* if the value can be exactly represented as a float, use float dedicated code otherwise generic */ + /* if the value can be exactly represented as a double, use double dedicated code otherwise generic */ if (min_dval > (double)Z_LVAL(args[i])) { min_dval = (double)Z_LVAL(args[i]); min = &args[i]; @@ -1339,7 +1339,7 @@ PHP_FUNCTION(max) max = &args[i]; } } else if (Z_TYPE(args[i]) == IS_DOUBLE && (zend_dval_to_lval((double) max_lval) == max_lval)) { - /* if max_level can be exactly represented as a float, go to float dedicated code */ + /* if max_lval can be exactly represented as a double, go to double dedicated code */ max_dval = (double) max_lval; goto double_compare; } else { @@ -1359,7 +1359,7 @@ PHP_FUNCTION(max) max = &args[i]; } } else if (Z_TYPE(args[i]) == IS_LONG && (zend_dval_to_lval((double) Z_LVAL(args[i])) == Z_LVAL(args[i]))) { - /* if the value can be exactly represented as a float, use float dedicated code otherwise generic */ + /* if the value can be exactly represented as a double, use double dedicated code otherwise generic */ if (max_dval < (double)Z_LVAL(args[i])) { max_dval = (double)Z_LVAL(args[i]); max = &args[i];