Fishwaldo opened a new pull request, #19856:
URL: https://github.com/apache/nuttx/pull/19856
## Summary
Four functions in the RISC-V machine directory decide whether to work a
register at a time by asking whether both pointers are already on a boundary.
That is the wrong question, and in one case it produces a store that the ISA
does not guarantee will work at all.
`strlcpy` aligns only its source and then stores a whole register at a time
to
a destination that is aligned by luck. `memcmp`, `strncmp` and `strcmp` test
`or` of the two pointers, so a pair that is equally misaligned falls back to
the byte loop the word loop exists to replace.
Two commits: the `strlcpy` store, then the three compares.
## Where this comes from
I opened #19735 with C implementations of these routines, built around
testing whether two pointers *agree* about where a boundary falls rather than
whether either is already on one. While that was in review, #19781 and #19782
landed assembly implementations covering a wider set of functions, so I have
dropped my versions; the assembly is the better base and is faster than my C
in most cases.
This PR carries over the one idea from #19735 that did not make it across,
and
applies it to the code that is now in tree. It is not a re-run of that
review.
I will close #19735 once this lands; the part of it that belongs in the
shared
BSD implementation rather than an arch directory, which is what
@xiaoxiang781216 asked for there, follows as its own PR.
## Why it matters more than a missed optimisation
Misaligned access is not guaranteed on RISC-V. The base ISA permits it to be
unsupported, and implementations differ:
- where firmware emulates it, every access traps into machine mode
- where nothing emulates it, the access faults
So a routine in a machine directory cannot assume a misaligned store will
work, whatever it might cost. `arch_strcpy.S` and `arch_memcpy.S` already
take
this view. `arch_strlcpy.S` does not.
**This is also why QEMU is a poor place to measure or test it.** QEMU
executes
misaligned accesses natively at full speed, so the `strlcpy` defect is
invisible there: correct results, no penalty, nothing to see. Every number
below is from silicon.
## Measured
EIC7700X, rv64 at 1.4 GHz, `CONFIG_RISCV_STRING_FUNCTION=y`, 32 KB operands,
taken with the benchmark in apache/nuttx-apps#3706. `generic` is the same
board with the machine directory disabled, included so the byte-loop rate is
visible.
Source and destination misaligned by different amounts, which is the case
`strlcpy` gets wrong:
```
generic before after
strlcpy 410.4 7.5 490.0 MB/s
```
7.5 MB/s is about 178 cycles per byte, flat from 512 bytes to 32 KB, which is
what a trapped and emulated store costs on this part. It is 55x slower than
the generic C it replaced.
Source and destination equally misaligned, which is the case the compares
reject:
```
generic before after
memcmp 30.7 34.4 456.0 MB/s
strncmp 32.2 32.2 254.0 MB/s
strcmp 41.2 40.9 280.0 MB/s
```
Each `before` figure is the generic rate, so the word loops were not being
entered at all.
Both aligned, to show the guard costs nothing where it does not fire:
```
before after
strlcpy 2474.0 2452.0 MB/s
memcmp 452.0 458.0 MB/s
strncmp 266.0 256.0 MB/s
strcmp 282.0 280.9 MB/s
```
Pointers that genuinely disagree still take the byte loop. No unaligned
access
is introduced anywhere.
## Testing
- Correctness: `testing/libc/arch_libc` under qemu rv64, all functions pass
before and after. The `strlcpy` defect is a performance defect on the parts
that emulate, so no correctness test can catch it, and none did.
- Throughput: the benchmark in apache/nuttx-apps#3706, which sweeps sizes
against every source and destination alignment pair. An aligned measurement
at a single size cannot see any of this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
