[PATCH 3/3] lib: Update the muldiv64 tests to verify the C on x86-64

David Laight posted 3 patches 10 months, 1 week ago
[PATCH 3/3] lib: Update the muldiv64 tests to verify the C on x86-64
Posted by David Laight 10 months, 1 week ago
div64.c contains a 128 by 64 division algorithm which x86-64 overrides
it with an asm implementation.
So running the muldiv64 tests only verifies the asm code.
Since x86-64 is the most likely test system compile the default
code into an x86-64 kernel (under a different name) when the tests
are being built.
Verify that both the asm and C functions generate the correct results.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 lib/math/div64.c                    | 18 ++++++++++++++++--
 lib/math/test_mul_u64_u64_div_u64.c | 26 ++++++++++++++++++++------
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/lib/math/div64.c b/lib/math/div64.c
index 50e025174495..38ee5c01c288 100644
--- a/lib/math/div64.c
+++ b/lib/math/div64.c
@@ -25,6 +25,8 @@
 #include <linux/minmax.h>
 #include <linux/log2.h>
 
+#include <generated/autoconf.h>
+
 /* Not needed on 64bit architectures */
 #if BITS_PER_LONG == 32
 
@@ -183,10 +185,22 @@ u32 iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
 }
 EXPORT_SYMBOL(iter_div_u64_rem);
 
-#if !defined(mul_u64_add_u64_div_u64)
+/*
+ * If the architecture overrides the implementation below and the test module
+ * is being built then compile the default implementation with a different name
+ * so that it can be tested.
+ */
+#if defined(mul_u64_add_u64_div_u64) && (defined(CONFIG_TEST_MULDIV64) || defined(CONFIG_TEST_MULDIV64_MODULE))
+#define TEST_MULDIV64
+#undef mul_u64_add_u64_div_u64
+#define mul_u64_add_u64_div_u64 mul_u64_add_u64_div_u64_test
+u64 mul_u64_add_u64_div_u64_test(u64 a, u64 b, u64 c, u64 d);
+#endif
+
+#if !defined( mul_u64_add_u64_div_u64) || defined(TEST_MULDIV64)
 u64 mul_u64_add_u64_div_u64(u64 a, u64 b, u64 c, u64 d)
 {
-#if defined(__SIZEOF_INT128__)
+#if defined(__SIZEOF_INT128__) && !defined(TEST_MULDIV64)
 
 	/* native 64x64=128 bits multiplication */
 	u128 prod = (u128)a * b + c;
diff --git a/lib/math/test_mul_u64_u64_div_u64.c b/lib/math/test_mul_u64_u64_div_u64.c
index 9548eb7458c7..e2289b412601 100644
--- a/lib/math/test_mul_u64_u64_div_u64.c
+++ b/lib/math/test_mul_u64_u64_div_u64.c
@@ -73,6 +73,10 @@ done
 
  */
 
+#ifdef mul_u64_add_u64_div_u64
+u64 mul_u64_add_u64_div_u64_test(u64 a, u64 b, u64 add, u64 c);
+#endif
+
 static int __init test_init(void)
 {
 	int errors = 0;
@@ -80,21 +84,31 @@ static int __init test_init(void)
 
 	pr_info("Starting mul_u64_u64_div_u64() test\n");
 
-	for (i = 0; i < ARRAY_SIZE(test_values); i++) {
-		u64 a = test_values[i].a;
-		u64 b = test_values[i].b;
-		u64 c = test_values[i].c;
-		u64 expected_result = test_values[i].result;
+	for (i = 0; i < ARRAY_SIZE(test_values) * 2; i++) {
+		u64 a = test_values[i / 2].a;
+		u64 b = test_values[i / 2].b;
+		u64 c = test_values[i / 2].c;
+		u64 expected_result = test_values[i / 2].result;
 		u64 result = mul_u64_u64_div_u64(a, b, c);
 		u64 result_up = mul_u64_u64_div_u64_roundup(a, b, c);
 
+#ifdef mul_u64_add_u64_div_u64
+		if (i & 1) {
+			/* Verify the generic C version */
+			result = mul_u64_add_u64_div_u64_test(a, b, 0, c);
+			result_up = mul_u64_add_u64_div_u64_test(a, b, c - 1, c);
+		}
+#else
+		i++;
+#endif
+
 		if (result != expected_result) {
 			pr_err("ERROR: 0x%016llx * 0x%016llx / 0x%016llx\n", a, b, c);
 			pr_err("ERROR: expected result: %016llx\n", expected_result);
 			pr_err("ERROR: obtained result: %016llx\n", result);
 			errors++;
 		}
-		expected_result += test_values[i].round_up;
+		expected_result += test_values[i / 2].round_up;
 		if (result_up != expected_result) {
 			pr_err("ERROR: 0x%016llx * 0x%016llx +/ 0x%016llx\n", a, b, c);
 			pr_err("ERROR: expected result: %016llx\n", expected_result);
-- 
2.39.5
Re: [PATCH 3/3] lib: Update the muldiv64 tests to verify the C on x86-64
Posted by Nicolas Pitre 10 months, 1 week ago
On Sat, 5 Apr 2025, David Laight wrote:

> div64.c contains a 128 by 64 division algorithm which x86-64 overrides
> it with an asm implementation.
> So running the muldiv64 tests only verifies the asm code.
> Since x86-64 is the most likely test system compile the default
> code into an x86-64 kernel (under a different name) when the tests
> are being built.
> Verify that both the asm and C functions generate the correct results.
> 
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>  lib/math/div64.c                    | 18 ++++++++++++++++--
>  lib/math/test_mul_u64_u64_div_u64.c | 26 ++++++++++++++++++++------
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/math/div64.c b/lib/math/div64.c
> index 50e025174495..38ee5c01c288 100644
> --- a/lib/math/div64.c
> +++ b/lib/math/div64.c
> @@ -25,6 +25,8 @@
>  #include <linux/minmax.h>
>  #include <linux/log2.h>
>  
> +#include <generated/autoconf.h>

Isn't this automatically included everywhere by the Makefile?

> @@ -183,10 +185,22 @@ u32 iter_div_u64_rem(u64 dividend, u32 divisor, u64 *remainder)
>  }
>  EXPORT_SYMBOL(iter_div_u64_rem);
>  
> -#if !defined(mul_u64_add_u64_div_u64)
> +/*
> + * If the architecture overrides the implementation below and the test module
> + * is being built then compile the default implementation with a different name
> + * so that it can be tested.
> + */
> +#if defined(mul_u64_add_u64_div_u64) && (defined(CONFIG_TEST_MULDIV64) || defined(CONFIG_TEST_MULDIV64_MODULE))

You could shorten this to:

#if defined(mul_u64_add_u64_div_u64) && IS_ENABLED(CONFIG_TEST_MULDIV64)

> +#define TEST_MULDIV64

Then I'd use IS_ENABLED(CONFIG_TEST_MULDIV64) in place of TEST_MULDIV64.
It is more self explanatory.

> +#undef mul_u64_add_u64_div_u64
> +#define mul_u64_add_u64_div_u64 mul_u64_add_u64_div_u64_test
> +u64 mul_u64_add_u64_div_u64_test(u64 a, u64 b, u64 c, u64 d);
> +#endif

Hmmm... I wish there could be a better way to do this, but other than 
the above suggestion I don't see one.

> +
> +#if !defined( mul_u64_add_u64_div_u64) || defined(TEST_MULDIV64)
>  u64 mul_u64_add_u64_div_u64(u64 a, u64 b, u64 c, u64 d)
>  {
> -#if defined(__SIZEOF_INT128__)
> +#if defined(__SIZEOF_INT128__) && !defined(TEST_MULDIV64)
>  
>  	/* native 64x64=128 bits multiplication */
>  	u128 prod = (u128)a * b + c;
> diff --git a/lib/math/test_mul_u64_u64_div_u64.c b/lib/math/test_mul_u64_u64_div_u64.c
> index 9548eb7458c7..e2289b412601 100644
> --- a/lib/math/test_mul_u64_u64_div_u64.c
> +++ b/lib/math/test_mul_u64_u64_div_u64.c
> @@ -73,6 +73,10 @@ done
>  
>   */
>  
> +#ifdef mul_u64_add_u64_div_u64
> +u64 mul_u64_add_u64_div_u64_test(u64 a, u64 b, u64 add, u64 c);
> +#endif
> +
>  static int __init test_init(void)
>  {
>  	int errors = 0;
> @@ -80,21 +84,31 @@ static int __init test_init(void)
>  
>  	pr_info("Starting mul_u64_u64_div_u64() test\n");
>  
> -	for (i = 0; i < ARRAY_SIZE(test_values); i++) {
> -		u64 a = test_values[i].a;
> -		u64 b = test_values[i].b;
> -		u64 c = test_values[i].c;
> -		u64 expected_result = test_values[i].result;
> +	for (i = 0; i < ARRAY_SIZE(test_values) * 2; i++) {
> +		u64 a = test_values[i / 2].a;
> +		u64 b = test_values[i / 2].b;
> +		u64 c = test_values[i / 2].c;
> +		u64 expected_result = test_values[i / 2].result;

I don't see the point of the loop doubling here.
If I understand it correctly, you'll test the default version twice and 
the _test version once for each test entry.


>  		u64 result = mul_u64_u64_div_u64(a, b, c);
>  		u64 result_up = mul_u64_u64_div_u64_roundup(a, b, c);
>  
> +#ifdef mul_u64_add_u64_div_u64
> +		if (i & 1) {
> +			/* Verify the generic C version */
> +			result = mul_u64_add_u64_div_u64_test(a, b, 0, c);
> +			result_up = mul_u64_add_u64_div_u64_test(a, b, c - 1, c);
> +		}
> +#else
> +		i++;
> +#endif
> +
>  		if (result != expected_result) {
>  			pr_err("ERROR: 0x%016llx * 0x%016llx / 0x%016llx\n", a, b, c);
>  			pr_err("ERROR: expected result: %016llx\n", expected_result);
>  			pr_err("ERROR: obtained result: %016llx\n", result);
>  			errors++;
>  		}
> -		expected_result += test_values[i].round_up;
> +		expected_result += test_values[i / 2].round_up;
>  		if (result_up != expected_result) {
>  			pr_err("ERROR: 0x%016llx * 0x%016llx +/ 0x%016llx\n", a, b, c);
>  			pr_err("ERROR: expected result: %016llx\n", expected_result);
> -- 
> 2.39.5
> 
>