memcpy c implementation#11
Conversation
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
newlib/libc/machine/riscv/memcpy.c:26
- Using 'char' for byte extraction can lead to sign extension issues on platforms where 'char' is signed. Consider using 'unsigned char' to ensure the byte values are correctly interpreted.
char b0 = *p++;
i3abghany
changed the title
i3abghany
force-pushed
the
memcpy_c_misaligned_access
branch
from
4ddb559 to
82defe7
Compare
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
newlib/libc/machine/riscv/memcpy.c:24
- [nitpick] The parameter name 'p' in __load_word is generic; consider renaming it to 'src' for improved clarity.
__load_word (const unsigned char *p)
newlib/libc/machine/riscv/memcpy.c:87
- [nitpick] The variable name 'la' is terse; consider using a more descriptive name (e.g., 'aligned_dest') to enhance readability.
long *la = (long *)a;
christian-herber-nxp
left a comment
christian-herber-nxp
left a comment
There was a problem hiding this comment.
small suggestion in comments.
i3abghany
force-pushed
the
memcpy_c_misaligned_access
branch
from
82defe7 to
8332de7
Compare
|
@christian-herber-nxp I think this looks good, so please let me know if you have other suggestions and if we can send a patch series soon. |
christian-herber-nxp
left a comment
christian-herber-nxp
left a comment
There was a problem hiding this comment.
looks pretty good.
for any reviewers and future readers sake, could you add some more comments explaining why certain pieces of code are the way they are.
Also, results on LLVM would still be nice.
Add a code path for when source and dest are differently aligned. If misaligned access is slow or prohibited, and the alignments of the source and destination are different, we align the destination to do XLEN stores. This uses only one aligned store for every four (or eight for XLEN == 64) bytes of data.
i3abghany
force-pushed
the
memcpy_c_misaligned_access
branch
from
8332de7 to
2323d35
Compare
Add a code path for when source and dest are differently aligned.
If misaligned access is slow or prohibited, and the alignments of the source and destination are different, we align the destination to do word stores. This uses only one aligned store for every four (or eight for XLEN == 64) bytes of data.
This only affects the case where source and destination are differently aligned. Previously, this case was handled by copying one byte at a time. The new approach significantly reduces the number of executed (dynamic) instructions (30% weighted average for -mstrict-align, while not really affecting -mno-strict-align):