Converting decimal to binary in Assembly

The longest example program in my PicoBlaze Simulator in JavaScript is this decimal-to-binary converter:

;This is an example program that uses
;UART, the interface that PicoBlaze uses
;for connecting to terminals (a DOS-like
;user interface, with a keyboard and a
;screen capable of displaying text).
;It loads base-10 integer numbers from
;the terminal, converts them into binary,
;and then prints the binary
;representations back onto the terminal.
;Example input would be:

;1
;2
;4
;8
;15
;127
;255
;

;And the expected output is:

;1_(10)=1_(2)
;2_(10)=10_(2)
;4_(10)=100_(2)
;8_(10)=1000_(2)
;15_(10)=1111_(2)
;127_(10)=1111111_(2)
;255_(10)=11111111_(2)
;

;Note that you need to click the
;"Enable UART" button in order to use it.
;Also, the trailing empty line in the
;input is necessary for the result to be
;printed.

;Now follows some boilerplate code
;we use in our Computer Architecture
;classes...
CONSTANT LED_PORT,00
CONSTANT HEX1_PORT,01
CONSTANT HEX2_PORT,02
CONSTANT UART_TX_PORT,03
CONSTANT UART_RESET_PORT,04
CONSTANT SW_PORT,00
CONSTANT BTN_PORT,01
CONSTANT UART_STATUS_PORT,02
CONSTANT UART_RX_PORT,03
; Tx data_present
CONSTANT U_TX_D, 00000001'b
; Tx FIFO half_full
CONSTANT U_TX_H, 00000010'b
; TxFIFO full
CONSTANT U_TX_F, 00000100'b
; Rxdata_present
CONSTANT U_RX_D, 00001000'b
; RxFIFO half_full
CONSTANT U_RX_H, 00010000'b
; RxFIFO full
CONSTANT U_RX_F, 00100000'b

START:
;At the beginning, the number is 0.
;And we are storing its string
;representation at the beginning
;of RAM.
namereg s3,pointer
;Now follows a loop to load
;the digits of the number.
;Load a character from the UART
;terminal.
call UART_RX
;Check whether the character is a digit.
compare s9,"0"
;part of the program for printing
;the number you have got.
jump c,print_the_number
compare s1,s9
jump c,print_the_number
;If it is a digit, store it into RAM.
store s9,(pointer)
;Multiply the number you have got by 10.
call multiply_by_10
;Then, convert the digit from ASCII
;into binary.
sub s9,"0"
;And then add it to the number you
;have got.
call c,abort ;In case of overflow.
;non-digit is
print_the_number:
;If there are no digits to be printed,
;do not print anything.
sub pointer,0
jump z,START
print_the_decimal:
printing_the_decimal_loop:
compare pointer,s4
jump nc, end_of_printing_the_decimal
fetch s9,(pointer)
;Do some basic sanity check: Is the
;character you are printing indeed
;a decimal digit?
compare s9,"0"
call c,abort
compare s1,s9
call c,abort
;If it is indeed a decimal digit,
;print it.
call UART_TX
jump printing_the_decimal_loop
end_of_printing_the_decimal:
;After you have repeated the decimal
;number, print the string "_(10)=".
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
;If the number to be printed is
;equal to zero, print 0.
sub s0,0
jump nz,print_the_binary
call UART_TX
jump end_of_printing_loop
print_the_binary:
;Make the pointer point to the
;beginning of RAM.
;Now goes a loop which stores the binary
;representation of the number we have
;got into RAM, but reversed.
beginning_of_converting_to_binary:
sub s0,0
jump z,end_of_converting_to_binary
sr0 s0
jump nc,store_digit_to_memory
store_digit_to_memory:
store s9,(pointer)
jump beginning_of_converting_to_binary
end_of_converting_to_binary:
;Do some basic sanity check, such as that
;the pointer does not point to zero.
compare pointer,0
call z,abort ;Something went wrong
;so end the program.
;Check whether there are more than 8 bits.
compare pointer,9
call nc,abort
;Now goes a loop which will print
;the binary number in RAM, with digits
;in the correct order. The pointer now
;points at a memory location right after
;the binary number (not at the last digit,
;but after it).
beginning_of_printing_loop:
sub pointer,1
jump c,end_of_printing_loop
fetch s9,(pointer)
;Do some basic sanity check:
;Is the character the pointer points to
;indeed a binary digit?
compare s9,"0"
jump z,memory_is_fine
compare s9,"1"
jump z,memory_is_fine
call abort ;Something went wrong,
;so end the program.
memory_is_fine:
;If everything is fine, print that
;digit.
call UART_TX
;Repeat until you have printed all
;digits of the binary number
;stored in RAM.
jump beginning_of_printing_loop
end_of_printing_loop:
;After you have printed that binary
;number, print the string "_(2)" and
;a new-line.
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
;The program runs in an infinite loop...
JUMP START

multiply_by_10:
call c,abort
call c,abort
call c,abort
call c,abort
return

abort:
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
call UART_TX
infinite_loop:
jump infinite_loop
return

;Now follows some boilerplate code
;we use in our Computer Architecture
;classes...
UART_RX:
INPUT sA, UART_STATUS_PORT
TEST sA, U_RX_D
JUMP Z, UART_RX
INPUT s9, UART_RX_PORT
RETURN

UART_TX:
INPUT sA, UART_STATUS_PORT
TEST sA, U_TX_F
JUMP NZ, UART_TX
OUTPUT s9, UART_TX_PORT
RETURN


So, what do you think about it? Do you have some suggestions about how to make it better?

You should insert more empty lines in order to cleanly separate the logical code blocks.

Your use of indentation is not all that consistent. It will improve readability a lot if you get this right.

Also, seeing that you can use some alias (namereg s3,pointer), I think you could use this more often to improve readability.

I feel your 'basic sanity checks' are more of 'insanity' checks. You should not doubt that much. The extra code also increases the risk of errors.

When something is wrong, you go to the abort part of the program never to return. It's confusing to see that most of the time you use a call to do this and not the much clearer jump.

Optimizations

If you're using the ASCII character set, then the colon character (":") follows the character "9". You can simplify the verification of a valid digit (no longer needing that temporary register s1) like:

compare s9,"0"
jump c,print_the_number
compare s9,":"
jump nc,print_the_number


Minimizing the number of instructions, and certainly control transferring instructions, is important. I have moved the more likely jumping back above the unlikely jumping to abort.

jump nc,loading_the_number ;Repeat until a non-digit is loaded.
jump abort

;If the number to be printed is
;equal to zero, print 0.
sub s0,0
jump nz,print_the_binary
call UART_TX
jump end_of_printing_loop


You don't need to special-case if the number happens to be zero. It will shave off numerous instructions.

Most of your loops are WHILE-WEND loops that have a condition on top and an unconditional jump that goes back to the top. Using REPEAT-UNTIL loops will be faster and require but one conditional jump instruction.

The new code:

START:
namereg s3,pointer

call UART_RX
compare s9,"0"
jump c,print_the_number
compare s9,":"
jump nc,print_the_number
store s9,(pointer)
call multiply_by_10
sub s9,"0"
jump abort

print_the_number:
sub pointer,0 ;If there are no digits to be printed, do not print anything.
jump z,START

printing_the_decimal_loop:
fetch s9,(pointer)
call UART_TX
compare pointer,s4
jump c,printing_the_decimal_loop

... some fixed text gets outputted here

load pointer,0                    ; Works even if s0 == 0
beginning_of_converting_to_binary:  ; This stores in reverse order!
sr0 s0
jump nc,store_digit_to_memory
store_digit_to_memory:
store s9,(pointer)
sub s0,0
jump nz,beginning_of_converting_to_binary

; pointer is certainly not zero
sub pointer,1
beginning_of_printing_loop:
fetch s9,(pointer)
call UART_TX
sub pointer,1
jump nc,beginning_of_printing_loop

... some fixed text gets outputted here

JUMP START