trjhtr

I am new to assembly and I want to know how I can improve this implementation both by speed and/or binary size optimization and making code to read and support easily.

Platform: Linux, x64

Assembler: NASM

Code
section .data

; Consts
%define XChar 'X'
%define OChar 'O'
%define filler '+'
%define LF 0x0a
%define separatorChars " -:,.;"

%define stdin 0x00
%define stdout 0x01

X: db XChar
O: db OChar
currentTurn: db XChar

; Buffers.
field: times 9 db filler

; Strings.
lineFeed: db LF
coordSeparators: db separatorChars
coordSeparatorsL: equ $ - coordSeparators

choosePlayerStr: db "Choose your character - type X or O: "
choosePlayerStrL: equ $ - choosePlayerStr
makeTurn: db 'Enter cell coord delimited by space: ',
makeTurnL: equ $ - makeTurn
boundsError: db 'Coords should be in range 1-3.', LF
boundsErrorL: equ $ - boundsError
cellUsedError: db 'Cell on specified coords is already used.', LF
cellUsedErrorL: equ $ - cellUsedError
separatorNotExistsError: db "Use one of '", separatorChars, "' characters between the coords.", LF
separatorNotExistsErrorL: equ $ - separatorNotExistsError
winStr: db " win!", LF
winStrL: equ $ - winStr
separatorStr: db "---------------------------", LF
separatorStrL: equ $ - separatorStr
drawStr: db "Draw!", LF
drawStrL: equ $ - drawStr

section .bss
 buf: resb 256 ; Buffer for user input.

section .text

%macro writeChar 1
 mov rax, 0x01
 mov rdi, stdout
 mov rsi, %1
 mov rdx, 0x01
 syscall
%endmacro

%macro writeConsole 1
 mov rax, 0x01
 mov rdi, stdout
 mov rsi, %1
 mov rdx, %1L
 syscall
%endmacro

%macro readConsole 0
 mov rax, 0x00
 mov rdi, stdin
 mov rsi, buf
 mov rdx, 256
 syscall
%endmacro

global _start
_start:
 call printField

chooseCharacter:
 writeConsole choosePlayerStr
 readConsole

 cmp byte [buf], XChar
 je characterChosen

 cmp byte [buf], OChar
 je characterChosen

 jmp chooseCharacter

characterChosen:
 mov al, [buf]
 mov byte [currentTurn], al

nextTurn:
 writeConsole makeTurn
 readConsole

 mov rcx, coordSeparatorsL
checkCoordSeparator:
 mov al, byte [buf + 1]
 cmp al, byte [rcx + coordSeparators - 1]
 je processUserInput

 dec rcx
 cmp rcx, 0
 jg checkCoordSeparator

 writeConsole separatorNotExistsError
 jmp nextTurn

processUserInput:
 movzx r8, byte [buf]
 movzx r9, byte [buf + 2]
 sub r8, '0'
 sub r9, '0'

 call makeMove

 ; Save move result (success/fail) in r12, because other registers will
 ; be used by printField of affected by syscall.
 mov r12, rax
 call printField

 ; If move was failed - make turn again.
 cmp r12, 0
 jne nextTurn

checkForDraw:
 mov rcx, 9 ; Max number of loops
 mov al, filler
 mov rdi, field
 repne scasb

 cmp rcx, 0
 je draw

 ; Check for win.
 ; 1. Check columns.
 xor cl, cl; Counter.

nextColumn:
 mov al, byte [rcx + field] ; Copy 1st cell to al.

 ; Cell shouldn't contain filler.
 cmp al, filler
 je endIteration

 cmp al, byte [rcx + field + 3]
 jne endIteration

 cmp al, byte [rcx + field + 6]
 je win

endIteration:
 inc rcx
 cmp rcx, 0x03
 jl nextColumn

 ; 2. Check rows.
 xor cl, cl ; Counter.

nextRow:
 mov al, byte [rcx * 3 + field] ; Copy 1st cell to al.

 ; Cell shouldn't contain filler.
 cmp al, filler
 je endRowIteration

 cmp al, byte [rcx * 3 + field + 1]
 jne endRowIteration

 cmp al, byte [rcx * 3 + field + 2]
 je win

endRowIteration:
 inc rcx
 cmp rcx, 0x03
 jl nextRow

 ; 3. Check diagonals.
 ; 3.1. Check 1st () diagonal.
 mov al, byte [field] ; Copy 1st cell to al.

 ; Cell shouldn't contain filler.
 cmp al, filler
 je secondDiagonalCheck

 cmp al, byte [field + 4]
 jne secondDiagonalCheck

 cmp al, byte [field + 8]
 je win

; 3.2. Check 2nd (/) diagonal.
secondDiagonalCheck:
 mov al, byte [field + 2] ; Copy 1st cell to al.

 ; Cell shouldn't contain filler.
 cmp al, filler
 je endDiagonalCheck

 cmp al, byte [field + 4]
 jne endDiagonalCheck

 cmp al, byte [field + 6]
 je win

endDiagonalCheck:
 ; Change current player.
 cmp byte [currentTurn], XChar
 cmove rax, [O]
 cmovne rax, [X]
 mov byte [currentTurn], al

 jmp nextTurn

draw:
 writeConsole separatorStr
 writeConsole drawStr
 jmp exit

win:
 writeConsole separatorStr
 writeChar currentTurn
 writeConsole winStr

exit:
 ; return 0
 mov rax, 60
 mov rdi, 0x00
 syscall

; ================================================
; void printField()
printField:
 xor bx, bx

printRow:
 mov rax, 0x01
 mov rdi, 0x01
 lea rsi, [ebx + ebx * 2 + field]
 mov rdx, 0x03
 syscall

 writeChar lineFeed

 inc bx
 cmp bx, byte 3
 jnz printRow

 ret
; ================================================

; ================================================
; makeMove(int8 row, int8 col)
;
; r8: row coord
; r9: column coord
makeMove:
 ; Indexing start with 0, then we should decrement both
 dec r8
 dec r9

 ; Checking bounds.
 cmp r8, 0
 jl outOfBoundsError
 cmp r9, 0
 jl outOfBoundsError
 cmp r8, 3
 jge outOfBoundsError
 cmp r9, 3
 jge outOfBoundsError

 ; Check if used.
 mov rsi, r8
 add rsi, r9
 cmp byte [rsi + r8*2 + field], filler
 jne usedCellError

 ; Set cell.
 mov rax, [currentTurn]
 mov byte [rsi + r8*2 + field], al

 xor rax, rax
 jmp makeMoveEnd

; Errors.
outOfBoundsError:
 writeConsole boundsError

 mov rax, 1
 jmp makeMoveEnd

usedCellError:
 writeConsole cellUsedError

 mov rax, 1
 jmp makeMoveEnd

makeMoveEnd:
 ret
; ================================================

Codeflow (not very detailed)

1) When zeroing registers, it takes less code to do xor eax, eax than mov eax, 0, and it runs (microscopically) faster. There's also a trick: While eax is usually just thought of as "the lower 32bits of rax," if you do xor eax, eax it implicitly zeros all 64 bits, and assembles to fewer bytes.

2) Looking at this code:

cmp byte [buf], OChar
je characterChosen

jmp chooseCharacter

characterChosen:

Consider restructuring it like this:

cmp byte [buf], OChar
jne chooseCharacter

characterChosen:

If it doesn't take the jne, it just continues on to the next statement. So doing things this way saves you from executing an extra jump.

3) Consider this code:

dec rcx
cmp rcx, 0
jg checkCoordSeparator

The way cmp works is that it sets some flags. When jg is executed, it looks at the current values of the flags to figure out what to do. But cmp isn't the only instruction that sets the flags. So does dec. Such being the case, you might try something like:

dec rcx
jnz checkCoordSeparator

Any time you find yourself doing a cmp, check to see if you have just done some math that would already set the flags for you so you can save time and not set them again.

For example sometimes you can restructure your loops: Instead of counting up from 0 to 3 (and comparing with 3), you can count down from 3 to 0 and use this trick. It does the same amount of work, but executes 1 fewer instruction.

4) I'm not sure what's in 'rcx' at that point, but it probably doesn't need the full 64bits that "rcx" can hold. If you don't expect the value to get bigger than about 2,147,483,647, using the 32bit version of instruction "ecx" can be a bit of a savings. Note that moving down to the 16bit version (cx) doesn't get more savings, and can be worse.

5) "I'm not sure what's in 'rcx' at that point" - And the reason I'm not sure, is that you don't have any comments there. Asm is particularly hard for humans to read. Instead of using names that humans can understand (ie "NumberOfMoves"), you get to use register names (ie 'rcx'). What's worse is that since there are only a few registers, you use them for a bunch of different things, so it's not always clear exactly what you are doing at any given point without reading all the code around it.

That's why well-written assembler is filled with comments; often every line ends with a comment saying what it's doing (something like this).

While it may be clear to you what this code does (since you just wrote it), when other people read your code (or if you read it again 6 months from now), it makes things much easier.

6) Then there's this:

cmp rcx, 0
je draw

When checking to see if a register is zero, this code is (slightly) smaller:

test rcx, rcx
je draw

It assembles to 1 fewer byte (2 fewer if you can use ecx instead of rcx).

Note that the suggestions above all (slightly) improve the size and (imperceptibly) improve the performance of your code, but they aren't going to change the results. It's not that what you are doing is going to give the wrong answer, it's just not the most efficient way. Oddly you seem to use some of these methods in some places and not others?

IAC, nowadays, it's almost always better to write your code in high level languages (like C) and let the compiler figure out all this stuff for you. They already know all these tricks (and a thousand more). Learning assembler is useful, but the more you learn, the more you realize that humans shouldn't be writing in assembler anymore.