| Message ID | 20230510035243.8586-1-zhang_fei_0403@163.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | riscv: Optimize memset for data sizes less than 16 bytes | expand |
From: zhangfei <zhangfei@nj.iscas.ac.cn> On Tue, May 09, 2023 11:16:33AM +0200, Andrew Jones wrote: > On Tue, May 09, 2023 at 10:22:07AM +0800, zhangfei wrote: > > > > Hi, > > > > I filled head and tail with minimal branching. Each conditional ensures that > > all the subsequently used offsets are well-defined and in the dest region. > > I know. You trimmed my comment, so I'll quote myself, here > > """ > After the check of a2 against 6 above we know that offsets 6(t0) > and -7(a3) are safe. Are we trying to avoid too may redundant > stores with these additional checks? > """ > > So, again. Why the additional check against 8 above and, the one you > trimmed, checking 10? Hi, These additional checks are to avoid too many redundant stores. Adding a check for more than 8 bytes is because after the loop segment '3' comes out, the remaining bytes are less than 8 bytes, which also avoids redundant stores. Thanks, Fei Zhang
On Wed, May 10, 2023 at 11:52:43AM +0800, zhangfei wrote: > From: zhangfei <zhangfei@nj.iscas.ac.cn> > > On Tue, May 09, 2023 11:16:33AM +0200, Andrew Jones wrote: > > On Tue, May 09, 2023 at 10:22:07AM +0800, zhangfei wrote: > > > > > > Hi, > > > > > > I filled head and tail with minimal branching. Each conditional ensures that > > > all the subsequently used offsets are well-defined and in the dest region. > > > > I know. You trimmed my comment, so I'll quote myself, here > > > > """ > > After the check of a2 against 6 above we know that offsets 6(t0) > > and -7(a3) are safe. Are we trying to avoid too may redundant > > stores with these additional checks? > > """ > > > > So, again. Why the additional check against 8 above and, the one you > > trimmed, checking 10? > > Hi, > > These additional checks are to avoid too many redundant stores. > > Adding a check for more than 8 bytes is because after the loop > segment '3' comes out, the remaining bytes are less than 8 bytes, > which also avoids redundant stores. So the benchmarks showed these additional checks were necessary to avoid making memset worse? Please add comments to the code explaining the purpose of the checks. Thanks, drew
From: zhangfei <zhangfei@nj.iscas.ac.cn> On Wed, May 10, 2023 at 14:58:22PM +0200, Andrew Jones wrote: > On Wed, May 10, 2023 at 11:52:43AM +0800, zhangfei wrote: > > From: zhangfei <zhangfei@nj.iscas.ac.cn> > > > > On Tue, May 09, 2023 11:16:33AM +0200, Andrew Jones wrote: > > > On Tue, May 09, 2023 at 10:22:07AM +0800, zhangfei wrote: > > > > > > > > Hi, > > > > > > > > I filled head and tail with minimal branching. Each conditional ensures that > > > > all the subsequently used offsets are well-defined and in the dest region. > > > > > > I know. You trimmed my comment, so I'll quote myself, here > > > > > > """ > > > After the check of a2 against 6 above we know that offsets 6(t0) > > > and -7(a3) are safe. Are we trying to avoid too may redundant > > > stores with these additional checks? > > > """ > > > > > > So, again. Why the additional check against 8 above and, the one you > > > trimmed, checking 10? > > > > Hi, > > > > These additional checks are to avoid too many redundant stores. > > > > Adding a check for more than 8 bytes is because after the loop > > segment '3' comes out, the remaining bytes are less than 8 bytes, > > which also avoids redundant stores. > > So the benchmarks showed these additional checks were necessary to avoid > making memset worse? Please add comments to the code explaining the > purpose of the checks. Hi, As you mentioned, the lack of these additional tests can make memset worse. When I removed the checks for 8 and 10 above, the benchmarks showed that the memset changed to 0.21 bytes/ns at 8B. Although this is better than storing byte by byte, additional detections will bring a better improvement to 0.27 bytes/ns. Due to the chaotic response in my previous email, I am sorry for this. I have reorganized patch v2 and sent it to you. Please reply under the latest patch. Thanks, Fei Zhang
From: zhangfei <zhangfei@nj.iscas.ac.cn> At present, the implementation of the memset function uses byte by byte storage when processing tail data or when the initial data size is less than 16 bytes. This approach is not efficient. Therefore, I filled head and tail with minimal branching. Each conditional ensures that all the subsequently used offsets are well-defined and in the dest region. Although this approach may result in redundant storage, compared to byte by byte storage, it allows storage instructions to be executed in parallel, reduces the number of jumps, and ultimately achieves performance improvement. I used the code linked below for performance testing and commented on the memset that calls the arm architecture in the code to ensure it runs properly on the risc-v platform. [1] https://github.com/ARM-software/optimized-routines/blob/master/string/bench/memset.c#L53 The testing platform selected RISC-V SiFive U74.The test data is as follows: Before optimization --------------------- Random memset (bytes/ns): memset_call 32K:0.45 64K:0.35 128K:0.30 256K:0.28 512K:0.27 1024K:0.25 avg 0.30 Medium memset (bytes/ns): memset_call 8B:0.18 16B:0.48 32B:0.91 64B:1.63 128B:2.71 256B:4.40 512B:5.67 Large memset (bytes/ns): memset_call 1K:6.62 2K:7.02 4K:7.46 8K:7.70 16K:7.82 32K:7.63 64K:1.40 After optimization --------------------- Random memset bytes/ns): memset_call 32K:0.46 64K:0.35 128K:0.30 256K:0.28 512K:0.27 1024K:0.25 avg 0.31 Medium memset (bytes/ns ) memset_call 8B:0.27 16B:0.48 32B:0.91 64B:1.64 128B:2.71 256B:4.40 512B:5.67 Large memset (bytes/ns): memset_call 1K:6.62 2K:7.02 4K:7.47 8K:7.71 16K:7.83 32K:7.63 64K:1.40 From the results, it can be seen that memset has significantly improved its performance with a data volume of around 8B, from 0.18 bytes/ns to 0.27 bytes/ns. Thanks, Fei Zhang Andrew Jones (1): RISC-V: lib: Improve memset assembler formatting arch/riscv/lib/memset.S | 143 ++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 71 deletions(-) zhangfei (1): riscv: Optimize memset arch/riscv/lib/memset.S | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-)