I apologize for my lack of GCC fluency in advance - I have only used MASM and RosAsm for x86, but I will try to translate. This review will be in top-to-bottom order, not in order of importance.
The first thing I would do is evaluate whether you really need to use cdecl calling convention. If you're only calling your function from asm, it makes sense to pass the source and destination in esi
and edi
, respectively, rather than putting them on the stack and then loading them.
Next, instead of movl %ecx, $0xFFFF
, I would do:
xorl %ecx, %ecx
decl %ecx
xor
ing a register with itself is such a common idiom that some processors use it as an optimization hint. All it does is set the register to zero, with a smaller opcode & operand. The dec
after the xor
makes the register all 1 bits no matter how many bits your register actually has. Some day when all of our GPRs are 128 bits, some poor sap that is updating assembly code will thank you for that =D.
Alternatively, you can forget about ecx
being a limit altogether. No matter what arbitrary limit you set on the size of the string, it will either (1) not be big enough for someone someday, or (2) be small enough that an access violation (or worse: no access violation) will occur before you actually reach that limit. Either way, that is really only a nominal protection of data integrity.
Now, the string. There seem to be some inconsistencies in how you're treating its terminating null character. You're using cmpl
to find four bytes of 0, then using movsb
to only copy/advance esi
by 1. Normally, strings are only guaranteed to be terminated by a number of null bytes equal to the character size, although in practice there are probably at least 2-3 to get the next datum to be dword-aligned. What that means for you is that your code will fail to detect the end of ~3/4 of normal, null-terminated, ascii strings, and keep copying until it finally causes an access violation.
But that's not all. Notice that you're fetching a dword at esi
with the cmpl
instruction, and that advancing that pointer by 1 at every iteration will make the pointer not dword-aligned 3/4 of the time. Loading non-aligned data takes two fetches instead of one, so for every 4 bytes of string, your cmpl
instruction alone needs 7 fetches from memory. Furthermore, after fetching the data and discarding it with cmpl
, you fetch it again with movsb
, a total of 11 memory loads per dword of data.
To reduce that number, you should load the data into a register, do your test for the null terminator on that register, then store the data to the destination. I see that someone else has pointed you to the bit-hack that will let you test all 4 bytes of the dword at once, so if you can follow that, do so, but here's a less efficient way that demonstrates my point very clearly:
cpy:
lodsb ; fetch one byte from [esi++] to al
test %al, %al ; set the z flag if al is 0
stosb ; store the byte, even if it's 0, doesn't affect flags
jnz cpy ; or loopnz if you still want to use a size limit in ecx
That will only require 4 fetches per dword, and is perfectly readable (as far as asm goes...). The bit twiddling hack does it with just one.
With regard to your instinct that rep
family instructions would be better, I believe you're sadly incorrect. If you learn any other assembly language and try to use strings, you'll certainly appreciate the effort Intel originally put into providing special string instructions, but they haven't really optimized or extended those instructions with the same zeal as one might like. Further, you can't repnz movsb
, because movsb
doesn't set any flags. If you ever do end up repne scasb
, remember to jecxz
=D.
This might seem like a long list of complaints, but for having first seen assembly 4 days before writing this, it's remarkable that you can do anything useful. Cheers.
loopne
instead of the jump? \$\endgroup\$