5
\$\begingroup\$

It takes in a string from the command line. prints out the length of the string. I'm mostly looking for tips and general convention suggestions
But I'm open to any other criticism also.
Roast it please thanks.

global _start

extern printf

section .data
    arg_err db `Invalid Arg Length\n\0`
    len_arg_err equ $ - arg_err
    arg_frmt db `%d\n\0`

section .text
_start:
    mov ecx, DWORD [esp]
    cmp ecx, 2
    jne _error_ext ; Args != 2? 

    mov eax,DWORD [esp+8] ;char** argv
    mov ecx, eax


_label:
    mov dl, byte [ecx]
    cmp dl, 0; read byte and comp to 0 
    je _exit
    add ecx, 1 
    jmp _label


_exit:
    sub ecx,eax ; subtract end addr from start addr
    push ecx
    push arg_frmt
    call printf 
    mov eax, 1
    int 0x80    


_error_ext:
    mov ecx, arg_err
    mov edx, len_arg_err
    call _print
    mov eax, 1
    int 0x80


_print: ; needs ecx as char* edx as char* len

    push eax
    push ebx

    mov eax, 4
    mov ebx, 1
    int 0x80
    pop ebx
    pop eax 
    retn

\$\endgroup\$
3
  • \$\begingroup\$ Explicit loops are usually suboptimal in x86 ISA. Consider repnz prefix. This answer is a good starting point. \$\endgroup\$
    – vnp
    Feb 25, 2020 at 5:26
  • \$\begingroup\$ strlen.asm:40: error: symbol `_print' undefined \$\endgroup\$ Feb 25, 2020 at 6:56
  • \$\begingroup\$ Welcome to Code Review! Please do not update the code in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ Feb 26, 2020 at 21:51

1 Answer 1

5
\$\begingroup\$
mov dl, byte [ecx]
cmp dl, 0; read byte and comp to 0 
je _exit

There are various alternatives, for example:

cmp byte [ecx], 0
je _exit

And:

mov dl, [ecx] ; note: BYTE would be redundant, but you can put it if you like
test dl, dl
jz _exit

For that loop overall,

_label:
    mov dl, byte [ecx]
    cmp dl, 0; read byte and comp to 0 
    je _exit
    add ecx, 1 
    jmp _label

This is a "2 jump loop", but it can be a "1 jump loop", for example:

_label:
    mov dl, [ecx]
    inc ecx
    test dl, dl
    jnz _label

Or:

    dec ecx
_label:
    inc ecx
    cmp byte [ecx], 0
    jne _label

(here are some details about inc vs add)

For fast loops like this, having two jumps in the loop can easily slow them down to half the speed.

repnz scasb is an option, but it's not that great. Small loops like this can run in 1 cycle per iteration on most CPUs anyway, and repnz scasb is not better than that (often worse, such as 2 cycles per byte on Ryzen and Haswel). The special optimizations for rep movs and rep stos don't apply to rep scas.

There are faster ways to implement strlen, that are not based on byte-by-byte loops but detect the presence of a zero inside a bigger block, using SIMD (example) or SWAR (example). That way speeds much higher than 1 byte per cycle can be reached. Perhaps some future processor would implement a "fast rep scas" feature in a similar way..

mov ecx, arg_err
mov edx, len_arg_err
call _print
int 0x80

This is mysterious, there is no _print function and there is a stray system call without selecting any particular entry point, or perhaps _print was meant to put a suitable value in eax?

\$\endgroup\$
4
  • \$\begingroup\$ It was another label I had defined I thought I switched everything over to use printf so I didn't include the function in my post. I'll edit the post when I get home today. It just set things up for the print syscall \$\endgroup\$
    – DNS_Jeezus
    Feb 25, 2020 at 15:07
  • \$\begingroup\$ I'm having trouble googling these "SIMD or SWAR" \$\endgroup\$
    – DNS_Jeezus
    Feb 25, 2020 at 22:13
  • 2
    \$\begingroup\$ @DNS_Jeezus I linked some examples. I could link the general concepts as well but I get good results if I google for them \$\endgroup\$
    – harold
    Feb 25, 2020 at 22:34
  • \$\begingroup\$ @DNS_Jeezus: One thing I'd add to this answer: movzx edx, byte [ecx] avoids a false dependency on the previous iteration. If you don't specifically want to merge a new low byte into the existing register value, use movzx for byte and word loads. It's just a simple zero-extending load like RISC machines do, not a load+merge. (uops.info and Why doesn't GCC use partial registers?) \$\endgroup\$ Dec 22, 2020 at 1:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.