Rewrite //sw/device/lib/base:memory_perftest in assembly to avoid brittle cycle counts
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
cc @cfrantz @alphan
Something in PR https://github.com/lowRISC/opentitan/pull/14044 invalidated `memory_perftest`'s hardcoded cycle count expectations. I updated the expectations in #14399, but left the test "flaky" and "manual" because we don't yet understand why it happened.
Disassembly diff: https://gist.github.com/dmcardle/4420de24287f77edf4642b5428d04878. This compares `//sw/device/lib/base:memory_perftest_prog_fpga_cw310_dis` from 4a22ede583c6b79d696f61c80c2dc2b2c84e295e to 3e99db1e33bbb4988011a9bdb21ab661f666c7e9.
I'm arbitrarily picking the "memcpy" test, which changed from 190290 to 210280 cycles. In theory, we only need to pay attention to code that is measured by the perftest, i.e. code that runs [between reads of the cycle counter](https://cs.opensource.google/opentitan/opentitan/+/master:sw/device/lib/base/memory_perftest.c;l=52;drc=ef077a26f4b65de702c31bceea8b38b448426304).
That means we'll start with `test_memcpy()`. Its code has not changed, except that it jumps to a slightly different address for `memcpy()`.
```diff
OT_NOINLINE void test_memcpy(uint8_t *buf1, uint8_t *buf2, size_t len) {
memcpy(buf1, buf2, len);
-2000080c: 6ec0106f j 20001ef8
+2000080c: 63e0106f j 20001e4a
```
Digging into `memcpy()`, the instructions and relative jumps are identical, but the entire function has been shifted to lower addresses. (This appears to have happened because `math` code like `udiv64_slow` used to come immediately before `memcpy`, but now it comes sometime after.)
```diff
-20001ef8 :
+20001e4a :
memcpy():
/proc/self/cwd/sw/device/lib/base/memory.c:23
OT_WEAK
@@ -5486,24 +5387,24 @@
uint8_t *dest8 = (uint8_t *)dest;
uint8_t *src8 = (uint8_t *)src;
for (size_t i = 0; i < len; ++i) {
-20001ef8: /----- ca11 beqz a2,20001f0c
-20001efa: | 86aa mv a3,a0
+20001e4a: /----- ca11 beqz a2,20001e5e
+20001e4c: | 86aa mv a3,a0
/proc/self/cwd/sw/device/lib/base/memory.c:24
dest8[i] = src8[i];
-20001efc: | /-> 00058703 lb a4,0(a1)
-20001f00: | | 00e68023 sb a4,0(a3)
+20001e4e: | /-> 00058703 lb a4,0(a1)
+20001e52: | | 00e68023 sb a4,0(a3)
/proc/self/cwd/sw/device/lib/base/memory.c:23
for (size_t i = 0; i < len; ++i) {
-20001f04: | | 167d addi a2,a2,-1
-20001f06: | | 0685 addi a3,a3,1
-20001f08: | | 0585 addi a1,a1,1
-20001f0a: | \-- fa6d bnez a2,20001efc
+20001e56: | | 167d addi a2,a2,-1
+20001e58: | | 0685 addi a3,a3,1
+20001e5a: | | 0585 addi a1,a1,1
+20001e5c: | \-- fa6d bnez a2,20001e4e
/proc/self/cwd/sw/device/lib/base/memory.c:26
}
return dest;
-20001f0c: \----> 8082 ret
+20001e5e: \----> 8082 ret
```
That's about as far as I can go with the disassembly diff since `memcpy` is a leaf node. I can't really explain how moving `test_memcpy` and `memcpy` closer to the test's call-site could make this test run slower. If anything, I'd expect more hits on the instruction cache.
@cfrantz Did you get further in your investigation?
Contributor guide
Assessment
This issue has not been assessed yet.