v6 is sent.
On 18.01.16 14:58, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Monday, January 18, 2016 2:24 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH v5] validation: system: add validation tests
for odp_cpu_cycles_* calls
On 18.01.16 13:37, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Ivan Khoronzhuk [mailto:ivan.khoronz...@linaro.org]
Sent: Friday, January 15, 2016 5:07 PM
To: lng-odp@lists.linaro.org
Cc: Savolainen, Petri (Nokia - FI/Espoo); Ivan Khoronzhuk
Subject: [lng-odp] [PATCH v5] validation: system: add validation tests
for
odp_cpu_cycles_* calls
https://bugs.linaro.org/show_bug.cgi?id=1906
Signed-off-by: Ivan Khoronzhuk <ivan.khoronz...@linaro.org>
---
test/validation/system/system.c | 130
++++++++++++++++++++++++++++++++++++++++
test/validation/system/system.h | 4 ++
2 files changed, 134 insertions(+)
diff --git a/test/validation/system/system.c
b/test/validation/system/system.c
index 7dc2cc0..f59c570 100644
--- a/test/validation/system/system.c
+++ b/test/validation/system/system.c
@@ -10,6 +10,9 @@
#include "test_debug.h"
#include "system.h"
+#define DIFF_TRY_NUM 160
+#define RES_TRY_NUM 10
+
void system_test_odp_version_numbers(void)
{
int char_ok = 0;
@@ -40,6 +43,129 @@ void system_test_odp_cpu_count(void)
CU_ASSERT(0 < cpus);
}
+void system_test_odp_cpu_cycles(void)
+{
+ uint64_t c2, c1;
+
+ c1 = odp_cpu_cycles();
+ odp_time_wait_ns(100);
+ c2 = odp_cpu_cycles();
+
+ CU_ASSERT(c2 != c1);
+}
+
+void system_test_odp_cpu_cycles_max(void)
+{
+ uint64_t c2, c1;
+ uint64_t max1, max2;
+
+ max1 = odp_cpu_cycles_max();
+ odp_time_wait_ns(100);
+ max2 = odp_cpu_cycles_max();
+
+ CU_ASSERT(max1 >= UINT32_MAX / 2);
+ CU_ASSERT(max1 == max2);
+
+ c1 = odp_cpu_cycles();
+ odp_time_wait_ns(1000);
+ c2 = odp_cpu_cycles();
+
+ CU_ASSERT(c1 <= max1 && c2 <= max1);
+}
+
+void system_test_odp_cpu_cycles_diff(void)
+{
+ int i;
+ uint64_t c2, c1, c3;
+ uint64_t tmp, diff;
+
+ c2 = 100;
+ c1 = 39;
+
+ diff = odp_cpu_cycles_diff(c2, c2);
+ CU_ASSERT(diff == 0);
+
+ tmp = c2 - c1;
+ diff = odp_cpu_cycles_diff(c2, c1);
+ CU_ASSERT(diff == tmp);
+
+ tmp = c1 + (odp_cpu_cycles_max() - c2) + 1;
As noted before, '+1' in the equation holds only if resolution is 1. The
equation should be:
diff = c2 + (odp_cpu_cycles_max() - c1) + odp_cpu_cycles_resolution();
It would be also more readable if c1 is always the first sample and c2
the second (c1 = 100, c2 = 39 in this test case).
In one case it's used directly in other in reverse order, no see reason to
assign it once again.
The reason is readability. Since it's not a performance target, it's better to assign
values again and highlight what is being tested (100 -> 39 vs. 39 -> 100).
Also, if reader compares the reference implementation to the test case, it's
confusing when c1 and c2 have been swapped.
return c2 + (odp_cpu_cycles_max() - c1) + 1 (the resolution);
VS.
tmp = c1 + (odp_cpu_cycles_max() - c2) + res;
-Petri
And I'll delete it at all and use the following instead:
/* check resolution for wrap */
c1 = max - 2 * res;
do
c2 = odp_cpu_cycles();
while (c2 < c1);
and use it for different cases:
diff = odp_cpu_cycles_diff(c1, c1);
CU_ASSERT(diff == 0);
tmp = c2 - c1;
// here c1 is supposed to be after c2, but in order to not read this
values again...
diff = odp_cpu_cycles_diff(c2, c1);
CU_ASSERT(diff == tmp);
rest = diff % res;
CU_ASSERT(rest == 0);
tmp = c1 + (odp_cpu_cycles_max() - c2) + res;
diff = odp_cpu_cycles_diff(c1, c2);
CU_ASSERT(diff == tmp);
rest = diff % res;
CU_ASSERT(rest == 0);
Max: 90
Res: 10
Valid values: 0, 10, 20, ..., 80, 90
c1 = 90
<wrap>
c2 = 0
c2 max c1 res
diff = 0 + (90 - 90) + 10 = 10
-Petri
+ diff = odp_cpu_cycles_diff(c1, c2);
+ CU_ASSERT(diff == tmp);
+
+ c3 = odp_cpu_cycles();
+
+ /* must handle one wrap */
+ for (i = 0; i < DIFF_TRY_NUM; i++) {
+ c1 = odp_cpu_cycles();
+ odp_time_wait_ns(100 * ODP_TIME_MSEC_IN_NS + i);
+ c2 = odp_cpu_cycles();
+
+ CU_ASSERT(c2 != c1);
+
+ if (c2 > c1)
+ tmp = c2 - c1;
+ else
+ tmp = c2 + (odp_cpu_cycles_max() - c1) + 1;
+
+ diff = odp_cpu_cycles_diff(c2, c1);
+ CU_ASSERT(diff == tmp);
+
+ /* wrap is detected and verified */
+ if (c2 < c1)
+ break;
+ }
+
+ /* wrap was detected, no need to continue */
+ if (i < DIFF_TRY_NUM)
+ return;
+
+ CU_ASSERT(odp_cpu_cycles_max() > UINT32_MAX);
+ CU_ASSERT((odp_cpu_cycles_max() - c3) > UINT32_MAX);
+
+ printf("wrap was not detected...");
+}
+
+void system_test_odp_cpu_cycles_resolution(void)
For this function I'll delete diff at all and move the test before diff
test.
+{
+ int i;
+ uint64_t rest;
+ uint64_t res, diff;
+ uint64_t c2, c1, max;
+
+ max = odp_cpu_cycles_max();
+
+ res = odp_cpu_cycles_resolution();
+ CU_ASSERT(res != 0);
+ CU_ASSERT(res < max / 1024);
+
+ /* check resolution for wrap */
+ c1 = max - 2 * res;
+ do
+ c2 = odp_cpu_cycles();
+ while (c1 < c2);
+
+ diff = odp_cpu_cycles_diff(c1, c2);
+ rest = diff % res;
+ CU_ASSERT(rest == 0);
+
+ for (i = 0; i < RES_TRY_NUM; i++) {
+ c1 = odp_cpu_cycles();
+ odp_time_wait_ns(100 * ODP_TIME_MSEC_IN_NS + i);
+ c2 = odp_cpu_cycles();
+
+ diff = odp_cpu_cycles_diff(c2, c1);
+ rest = diff % res;
+ CU_ASSERT(rest == 0);
+
+ rest = c1 % res;
+ CU_ASSERT(rest == 0);
+
+ rest = c2 % res;
+ CU_ASSERT(rest == 0);
+ }
+}
+
void system_test_odp_sys_cache_line_size(void)
{
uint64_t cache_size;
@@ -91,6 +217,10 @@ odp_testinfo_t system_suite[] = {
ODP_TEST_INFO(system_test_odp_sys_page_size),
ODP_TEST_INFO(system_test_odp_sys_huge_page_size),
ODP_TEST_INFO(system_test_odp_sys_cpu_hz),
+ ODP_TEST_INFO(system_test_odp_cpu_cycles),
+ ODP_TEST_INFO(system_test_odp_cpu_cycles_max),
+ ODP_TEST_INFO(system_test_odp_cpu_cycles_diff),
+ ODP_TEST_INFO(system_test_odp_cpu_cycles_resolution),
ODP_TEST_INFO_NULL,
};
diff --git a/test/validation/system/system.h
b/test/validation/system/system.h
index 869aaff..0c263f2 100644
--- a/test/validation/system/system.h
+++ b/test/validation/system/system.h
@@ -17,6 +17,10 @@ void system_test_odp_sys_cpu_model_str(void);
void system_test_odp_sys_page_size(void);
void system_test_odp_sys_huge_page_size(void);
void system_test_odp_sys_cpu_hz(void);
+void system_test_odp_cpu_cycles_max(void);
+void system_test_odp_cpu_cycles(void);
+void system_test_odp_cpu_cycles_diff(void);
+void system_test_odp_cpu_cycles_resolution(void);
/* test arrays: */
extern odp_testinfo_t system_suite[];
--
1.9.1
--
Regards,
Ivan Khoronzhuk
--
Regards,
Ivan Khoronzhuk
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
