patternModerate
Assembly beginner exercise: Calculate the square numbers from 1 to n
Viewed 0 times
thenumbersexercisebeginnerassemblycalculatesquarefrom
Problem
I try to learn (flat-, FASM-) Assembly currently.
For to become more familiar with the concept of branching I made a tiny program. Inspired by an exercise I had to do once in school when learning PHP.
"Write a program which takes an integer given by the user. The program then calculates and prints the square numbers from 1 to the given number."
Here my source code.
I have added comments in the code. Trying to explain my thoughts and motivations.
It seems to work as expected.
But I'm sure there's a lot of room for improvements ...
So therefore: Hints and comments from more experienced Assembly programmers very appreciated. :)
For to become more familiar with the concept of branching I made a tiny program. Inspired by an exercise I had to do once in school when learning PHP.
"Write a program which takes an integer given by the user. The program then calculates and prints the square numbers from 1 to the given number."
Here my source code.
I have added comments in the code. Trying to explain my thoughts and motivations.
format PE console
entry start
include 'win32a.inc'
; ===========================================
; = Displays the square number from 1 until =
; = number n. n is given by the user. =
; = @param [ integer ] n - The upper =
; = limit. The last integer to calculate =
; = the square number of. =
; ===========================================
; - Example --------------------------------
; 100 // User have entered 100.
; 1
; 4
; 9
; 10 // 16 in base 10
; 19 // 25 in base 10
; 24 // 36 in base 10
; 31 // ...
; 40
; 51
; 64
; 79
section '.text' code readable executable
start:
call read_hex ; Provided by teacher. Reads in a hexadecimal number from stdin.
mov ecx, eax
mov ebx, 1 ; First number ...
cmp eax, 1000000 ; When the number received is smaller the upper limit ...
jle createSquareNumbers ; ... then you do not have to override it.
mov ecx, ebp ; ... otherwise you override it with the upper limit.
createSquareNumbers:
mov edx, 0
mov eax, ebx
mul ebx ; Multiply the number with itself.
call print_eax ; ; Provided by teacher. Prints eax to stdin.
inc ebx ; Go to the next integer.
cmp ebx, ecx
jle createSquareNumbers
push 0
call [ExitProcess]
include 'training.inc'It seems to work as expected.
But I'm sure there's a lot of room for improvements ...
So therefore: Hints and comments from more experienced Assembly programmers very appreciated. :)
Solution
The biggest problem here—and generally with assembly—is the use of registers in the code without explicit documentation of what those registers contain, e.g., in the form of comments. Certain assumptions can be made based on common calling conventions (like that functions will return their result in
I further assume that
If you are targeting the Pentium Pro or later (so basically, any modern x86 processor) you can use conditional move instructions like so:
In some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the
This is completely correct code for setting a register to 0. However, it is not idiomatic x86 assembly because it is inefficient. Both compilers and expert assembly-language programmers will instead write this as:
This works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster.
The only time you would ever want to use
I can tell that you're working with signed integer values because you're using the
There is, unfortunately, one bug lying in wait here:
You are only checking the upper bound, so what happens when the input value is 0? Your code will print
And while we're nitpicking, some of your comments have "bugs", too:
-
That should be
-
Your function does not actually take a parameter. Rather, it retrieves the input by calling the
eax), but that should still be documented for clarity. In other cases, it is not quite so obvious. For example, in this code, ebp apparently contains the "upper limit", but other than assuming that the code works and reverse-engineering its logic, I have no basis for that assumption. In fact, in the current implementation, the contents of the ebp register are a pre-condition for the function, so its use should unarguably be documented as part of the function's signature.I further assume that
ebp is actually just 1000000, in which case this portion of your code is overly verbose (and thus inefficient). You can simply do:cmp eax, 1000000 ; see if input is over the upper limit
jle createSquareNumbers ; if not, skip the next line
mov eax, 1000000 ; if it is, bound itIf you are targeting the Pentium Pro or later (so basically, any modern x86 processor) you can use conditional move instructions like so:
cmp eax, ebp
cmovg eax, ebpIn some cases, conditional moves will make the code faster by eliminating the possibility of branch mispredictions. In this case, you won't see any performance improvement, but these instructions are still something to be aware of and this does make the code slightly easier to read. (Assuming that you want to keep the dependency on the
ebp register, which might be handy if you want to make the upper limit variable. Otherwise, just hard-code the 1000000 and use the first version. Better yet, define a constant for that upper bound so you can give it a readable name.)mov edx, 0This is completely correct code for setting a register to 0. However, it is not idiomatic x86 assembly because it is inefficient. Both compilers and expert assembly-language programmers will instead write this as:
xor edx, edxThis works because bitwise XORing any value with itself gives you back 0. The advantage is that it is shorter (2 bytes instead of 5 bytes) and faster.
The only time you would ever want to use
mov reg, 0 is if you didn't want to clobber the flags, which is handy when you're preparing to do a conditional move and in certain other tightly hand-optimized code. Otherwise, whenever you want to clear a register, think xor reg, reg. While this is arguably a micro-optimization, you really do need to know this idiom so you can read other peoples' (and compilers') assembly code, so you might as well use it yourself.I can tell that you're working with signed integer values because you're using the
l and g condition codes. If you were working with unsigned integer values, you'd be using the b and a condition codes. Therefore, you should be doing a signed integer multiply (imul instead of mul). In fact, imul is preferable for a number of reasons, chief among them is the fact that it has a normal, two-operand form that works just like any other instruction, which means you don't have to worry about the hard-coded register operands that mul requires. This makes the code easier to optimize and, frankly, easier to read. You can use imul for unsigned multiplications, too, as long as you aren't worried about overflow, since the lower 32 bits are always the same. (And clearly you aren't worried about that here, since your print_eax function used to display the output can only print 32-bit values!)There is, unfortunately, one bug lying in wait here:
You are only checking the upper bound, so what happens when the input value is 0? Your code will print
1, when instead it should print nothing! Also, I wonder if it is possible for read_hex to return a negative value? I don't have the function's implementation, or any documentation for it, so I can't tell. If you're not confident that it won't ever return a negative value, your code needs to handle this case. Fortunately, you can fix both of these potential problems easily by adding a check for the lower bound before entering the loop.And while we're nitpicking, some of your comments have "bugs", too:
-
call print_eax ; ; Provided by teacher. Prints eax to stdin.That should be
stdout, not stdin! :-)-
; ===========================================
; = Displays the square number from 1 until =
; = number n. n is given by the user. =
; = @param [ integer ] n - The upper =
; = limit. The last integer to calculate =
; = the square number of. =
; ===========================================Your function does not actually take a parameter. Rather, it retrieves the input by calling the
read_hex function. Therefore, this documentation is wrong, because it suggests that the function should be able toCode Snippets
cmp eax, 1000000 ; see if input is over the upper limit
jle createSquareNumbers ; if not, skip the next line
mov eax, 1000000 ; if it is, bound itcmp eax, ebp
cmovg eax, ebpxor edx, edxcall print_eax ; ; Provided by teacher. Prints eax to stdin.; ===========================================
; = Displays the square number from 1 until =
; = number n. n is given by the user. =
; = @param [ integer ] n - The upper =
; = limit. The last integer to calculate =
; = the square number of. =
; ===========================================Context
StackExchange Code Review Q#155155, answer score: 11
Revisions (0)
No revisions yet.