Why does this assembly language program print hex digits in reverse order?

Question

I've been following lecture notes on how to write an operating system and have been getting to grips with assembly language, specifically NASM. (Lecture notes here, for interest: https://www.cs.bham.ac.uk/~exr/lectures/opsys/10_11/lectures/os-dev.pdf)

One of the first tasks is to write a program that will print to the screen an ASCII representation of a 16-bit hexadecimal number.

In the program below, the test number is '0x6bf1'. The program prints the number, but with the hex digits reversed, i.e. '01fb6'. I cannot figure out why - would someone be able to give me a hint? (This isn't homework btw).

[org 0x7c00]            ; BIOS loads bootloader to address 0x7c00

mov dx, 0x6bf1
call print_hex
jmp $                    ; Hang after printing result

print_hex:

mov cl, 0
mov bx, HEX_OUT
add bx, 2            ; To start writing after '0x'

loop:

cmp cl, 16
je finally
mov ax, dx
shr ax, cl
and ax, 0x000f
add ax, 0x30
cmp ax, 0x39
jg add_7
mov byte [bx], al
add bx, 1            ; Increment write address for the next round
add cl, 4            ; Increment bit shift for the next round
jmp loop

add_7:               ; Handles letters (A-F)

add ax, 0x07
mov byte [bx], al
add bx, 1            ; Increment write address for the next round
add cl, 4            ; Increment bit shift for the next round
jmp loop


finally:

mov bx, HEX_OUT
call print_string
ret

print_string:
mov ah, 0x0e         ; Set up for BIOS Teletype Routine
mov dx, bx

print_loop:
mov cl, [bx]
cmp cl, 0
je exit
mov al, [bx]
int 0x10
add bx, 1
jmp print_loop

exit:
ret

HEX_OUT: db '0x0000',0

; padding and magic BIOS number
times 510-($-$$) db 0
dw 0xaa55

Answer 1

Thanks to the comments by Michael Petch and Margaret Bloom you already know what was wrong with your code. The solution is to have a loop like:

    mov  cl, 12
Loop:
    mov  ax, dx
    shr  ax, cl
    ...
    sub  cl, 4
    jnb  Loop

---

Since this isn't homework we can go a bit further and learn to write better code. The body of the main loop is inefficient because you repeat a number of instructions where this isn't necessary. See how much smaller the code gets by just jumping over the addition with 7:

    mov  cl, 12
Loop:
    mov  ax, dx
    shr  ax, cl
    and  al, 0x0F
    add  al, "0"
    cmp  al, "9"
    jng  isDigit
    add  al, 7         ; Handles letters (A-F)
isDigit:
    mov  byte [bx], al
    inc  bx            ; Increment write address for the next round
    sub  cl, 4
    jnb  Loop

Please note that I used AL instead of AX. The upper byte of AX at these points in the program has no relevant content. Furthermore using AL reduces the code size. Using inc bx rather than add bx,1 also reduces code size.

The BIOS Teletype function has BH as an argument, so best not use BX to iterate over the string when outputting the result.
Why do you load the character 2 times (once in CL, once in AL)?
See how I've put the test for the end of the string near the end of the loop? This saves unnecessary jumping around. In future programs that you'll write this will become an important detail.

print_string:
    mov  si, bx
    mov  ah, 0x0E      ; Set up for BIOS Teletype Routine - function number
    mov  bh, 0         ; Set up for BIOS Teletype Routine - display page
    jmp  TestEnd
print_loop:
    int  0x10
    inc  si
TestEnd:
    mov  al, [si]
    cmp  al, 0
    jne  print_loop
    ret