Skip to main content
Code Review

Return to Answer

Added hyperlink; added formatting; fixed typos
Source Link
Jamal
Jamal
  • 34.9k
  • 13
  • 133
  • 237

Since the result register (EAXEAX) is 32-bit, should I take into account a possible result that may not fit in 32 bits? If I get a larger result, it won't display properly, and the user may be unaware.

Is doing a bunch of adds still faster than using mulmul?

http://www2.math.uni-wuppertal.de/~fpf/Uebungen/GdR-SS02/opcode_i.htmlThis suggests that on a Pentium, ADDADD is about 3 cycles whereas MULMUL is about 10 cycles. So MULMUL is faster if you need to multiply by more than about 3 or 4.

I'm also avoiding mulmul because getting the bits in the right place for it is complicated, and I still cannot seem to get it right.

Perhaps ask about this on StackOverflowStack Overflow.

  • Programmers who use high-level-languages are used to them; a nest of jump are "spaghetti code" and "goto is considered harmful" (compared with 'structured programming')
  • A subroutine can be called from more than one place (i.e. reused)
  • One of the problems with assembly is knowing what registers are used. You documented which registers are used by your code fragments. For larger programs you might want to define a standard used by all subroutine, for example, "any subroutine may use/corrupt eaxeax through edx;edx; if it alters any other register (e.g. esiesi, ebpebp, etc.) then it must save previous value before changing it and retorerestore old value after changing it before returning.
; assume the following calling cnventionsconventions:
; esi is the input parameter
; eax is the output value
; carry flag is set if there's an error
mov esi, input_lbl
call PutStr
call GetUint
jnc input_ok
mov esi, input_error
call PutStr
.EXIT
input_ok:
mov esi, eax
call factorial
jnc output_ok
mov esi, factorial_overflow_error
call PutStr
.EXIT
output_ok
push eax ; preserve the factorial value
mov esi, factorial_overflow_is
call PutStr
pop esi ; restore the factorial value
call PutUint
.EXIT

If GetLIntGetLInt is a macro would it work to just say GetLInt EAX?

Since the result register (EAX) is 32-bit, should I take into account a possible result that may not fit in 32 bits? If I get a larger result, it won't display properly, and the user may be unaware.

Is doing a bunch of adds still faster than using mul?

http://www2.math.uni-wuppertal.de/~fpf/Uebungen/GdR-SS02/opcode_i.html suggests that on a Pentium, ADD is about 3 cycles whereas MUL is about 10 cycles. So MUL is faster if you need to multiply by more than about 3 or 4.

I'm also avoiding mul because getting the bits in the right place for it is complicated, and I still cannot seem to get it right.

Perhaps ask about this on StackOverflow.

  • Programmers who use high-level-languages are used to them; a nest of jump are "spaghetti code" and "goto is considered harmful" (compared with 'structured programming')
  • A subroutine can be called from more than one place (i.e. reused)
  • One of the problems with assembly is knowing what registers are used. You documented which registers are used by your code fragments. For larger programs you might want to define a standard used by all subroutine, for example, "any subroutine may use/corrupt eax through edx; if it alters any other register (e.g. esi, ebp, etc) then it must save previous value before changing it and retore old value after changing it before returning.
; assume the following calling cnventions:
; esi is the input parameter
; eax is the output value
; carry flag is set if there's an error
mov esi, input_lbl
call PutStr
call GetUint
jnc input_ok
mov esi, input_error
call PutStr
.EXIT
input_ok:
mov esi, eax
call factorial
jnc output_ok
mov esi, factorial_overflow_error
call PutStr
.EXIT
output_ok
push eax ; preserve the factorial value
mov esi, factorial_overflow_is
call PutStr
pop esi ; restore the factorial value
call PutUint
.EXIT

If GetLInt is a macro would it work to just say GetLInt EAX?

Since the result register (EAX) is 32-bit, should I take into account a possible result that may not fit in 32 bits? If I get a larger result, it won't display properly, and the user may be unaware.

Is doing a bunch of adds still faster than using mul?

This suggests that on a Pentium, ADD is about 3 cycles whereas MUL is about 10 cycles. So MUL is faster if you need to multiply by more than about 3 or 4.

I'm also avoiding mul because getting the bits in the right place for it is complicated, and I still cannot seem to get it right.

Perhaps ask about this on Stack Overflow.

  • Programmers who use high-level-languages are used to them; a nest of jump are "spaghetti code" and "goto is considered harmful" (compared with 'structured programming')
  • A subroutine can be called from more than one place (i.e. reused)
  • One of the problems with assembly is knowing what registers are used. You documented which registers are used by your code fragments. For larger programs you might want to define a standard used by all subroutine, for example, "any subroutine may use/corrupt eax through edx; if it alters any other register (e.g. esi, ebp, etc.) then it must save previous value before changing it and restore old value after changing it before returning.
; assume the following calling conventions:
; esi is the input parameter
; eax is the output value
; carry flag is set if there's an error
mov esi, input_lbl
call PutStr
call GetUint
jnc input_ok
mov esi, input_error
call PutStr
.EXIT
input_ok:
mov esi, eax
call factorial
jnc output_ok
mov esi, factorial_overflow_error
call PutStr
.EXIT
output_ok
push eax ; preserve the factorial value
mov esi, factorial_overflow_is
call PutStr
pop esi ; restore the factorial value
call PutUint
.EXIT

If GetLInt is a macro would it work to just say GetLInt EAX?

added 703 characters in body
Source Link
ChrisW
ChrisW
  • 12.9k
  • 1
  • 34
  • 76

For example your program might look like this if it were written with subroutines:

; assume the following calling cnventions:
; esi is the input parameter
; eax is the output value
; carry flag is set if there's an error
mov esi, input_lbl
call PutStr
call GetUint
jnc input_ok
mov esi, input_error
call PutStr
.EXIT
input_ok:
mov esi, eax
call factorial
jnc output_ok
mov esi, factorial_overflow_error
call PutStr
.EXIT
output_ok
push eax ; preserve the factorial value
mov esi, factorial_overflow_is
call PutStr
pop esi ; restore the factorial value
call PutUint
.EXIT

For example your program might look like this if it were written with subroutines:

; assume the following calling cnventions:
; esi is the input parameter
; eax is the output value
; carry flag is set if there's an error
mov esi, input_lbl
call PutStr
call GetUint
jnc input_ok
mov esi, input_error
call PutStr
.EXIT
input_ok:
mov esi, eax
call factorial
jnc output_ok
mov esi, factorial_overflow_error
call PutStr
.EXIT
output_ok
push eax ; preserve the factorial value
mov esi, factorial_overflow_is
call PutStr
pop esi ; restore the factorial value
call PutUint
.EXIT
Source Link
ChrisW
ChrisW
  • 12.9k
  • 1
  • 34
  • 76

Is my input validation reducing readability, or is that not important as long as the program proceeds with only the correct input?

Usually, "does it implement required functionality?" is even more important than "is it readable?"

A "Hello world" program is very readable but doesn't implement required functionality.

Should I need to check for non-numerical values, or does it not really matter?

When you're programming for other people (e.g. professionally) there might be someone other than the programmer (e.g. there's a boss, a product manager, a UI designer) who specifies what is implemented: and your job as programmer is to decide how to implement that.

Whether validating input "matters" depends on why you're writing the program, on who you're writing it for, and on who is going to use it.

Since the result register (EAX) is 32-bit, should I take into account a possible result that may not fit in 32 bits? If I get a larger result, it won't display properly, and the user may be unaware.

Again that's a matter of the program's "requirements". Different programs have different requirements.

IIRC a 32-bit number is big enough for 13! but not 14!.

Is doing a bunch of adds still faster than using mul?

http://www2.math.uni-wuppertal.de/~fpf/Uebungen/GdR-SS02/opcode_i.html suggests that on a Pentium, ADD is about 3 cycles whereas MUL is about 10 cycles. So MUL is faster if you need to multiply by more than about 3 or 4.

I'm also avoiding mul because getting the bits in the right place for it is complicated, and I still cannot seem to get it right.

Perhaps ask about this on StackOverflow.

Should any of these procedures be organized differently? I'm still not sure how important this is in assembly when when most procedures have jumps or branches. In this program, it makes sense to have check_input after get_input and with no jump in between them.

I suppose there are three reasons to use subroutines:

  • Programmers who use high-level-languages are used to them; a nest of jump are "spaghetti code" and "goto is considered harmful" (compared with 'structured programming')
  • A subroutine can be called from more than one place (i.e. reused)
  • One of the problems with assembly is knowing what registers are used. You documented which registers are used by your code fragments. For larger programs you might want to define a standard used by all subroutine, for example, "any subroutine may use/corrupt eax through edx; if it alters any other register (e.g. esi, ebp, etc) then it must save previous value before changing it and retore old value after changing it before returning.
GetLInt   [input]
mov       EAX, [input]

If GetLInt is a macro would it work to just say GetLInt EAX?

mov       EBX, [input]

mov EBX, EAX (i.e. move from register) would be faster and shorter than move from memory.

"Number must be positive!", 0

EBX contains bits. Those bits can represent a signed or an unsigned number. You might like to use a different macro e.g. GetLUInt which errors if the user enters a - minus sign, and accepts unsigned integers e.g. 4000000000.

nwln

Is this a macro not an opcode? It might be better to use e.g. upper case for macro names and lower case for opcode names.