patternMinor
Calculating prime factors in MIPS assembly
Viewed 0 times
factorsmipscalculatingassemblyprime
Problem
The goal of this was to be able to enter any number and print out the prime factors of that number. Are there any shorter ways of writing this code? I'm not very familiar with low-level syntax and was hoping somebody could give it a look.
.text
.align 2
.globl main
main:
# compute and display the first prime numbers up to n where n is given. Set n to be 19 but the program should work of any value of n.
li $v0, 4
la $a0, prompt
syscall # call operating sys
li $v0, 5 # read keyboard into $v0 (number n is the limit)
syscall # call operating sys
move $t0,$v0 # store n in $t0
li $v0, 4 # display message to user
la $a0, message
syscall
# call operating sys
li $v0, 4
la $a0, space
syscall # call operating sys
# Load variables into registers
lw $t1, i # $t1 = i
lw $t2, k # $t2 = k
lw $t3, p # $t3 = p
lw $t5, c # $t5 = c
lw $t6, d # $t6 = d
blt $t0,$t1 L2
li $v0, 1 # print integer function call 1
move $a0, $t1 # integer to print
syscall
# call operating sys
li $v0, 4 # print new line
la $a0, space
syscall # call operating sys
#Outer loop
L2: move $t3, $zero
# Inner loop
L1: remu $t4, $t1, $t2
bne $t4, $zero, I
move $t3, $t5
I: add $t2, $t2, $t5 # increment k
blt $t2, $t1 L1
bne $t3, $zero, P
li $v0, 1 # print integer function call 1
move $a0, $t1 # integer to print
syscall
li $v0, 4 # print new line
la $a0, space
syscall # call operating sys
P: add $t1, $t1, $t5 # increment i
move $t2, $t6
bgt $t1, $t0, E
j L2
E: li $v0, 10 # system call code for exit = 10
syscall # call operating sys
end: jr $ra
.data
prompt:
.asciiz "Please enter the number you wish to find the prime numbers of : "
message:
.asciiz "The Prime Numbers are : "
space: .asciiz "\n "
#initialization of registers
i:
.word 2
p:
.word 0
k:
.word 2
c:
.word 1
d:
.word 2Solution
Procedures
-
You appear to be doing everything in
-
Since
Structure
-
The register initializations should be placed above
-
The loop labels barely stand out, and this really hurts structure. The loop procedures are all crammed together and
-
The procedures should be properly indented to help them stay isolated.
All of that could should be indented together and with
Comments
-
The side comments here seem redundant:
The top comment describes this block clearly enough, whether or not the reader already knows that
-
This comment is also redundant:
Unless there are some special circumstances to note, such comments do not add anything important. Readers familiar with MIPS commands will already know what this means. You may just mention this at the top of the program if you'd like for anyone to know what this means. Putting it with every call just clutters your code.
This, on the other hand, doesn't state the obvious:
Without the comment, all I can tell is that this is equivalent to
Overall, this is just very hard to follow. If there are bugs in here, it would be difficult to isolate them. This, in turn, hurts maintainability. Comments alone can't help with this, either. The overall structure must be sound, otherwise fixing problems is just a matter of picking apart separate lines, one-by-one.
-
You appear to be doing everything in
main while just branching into single-character procedures. Consider renaming them to specify their purpose, while separating them from main with proper indentation and such. I cannot tell what main should be doing by itself. I can just see that it prompts for user input and operates the two loops with its procedures.-
Since
prompt is done first, consider defining it above main instead. The end of the program should just display the result(s).Structure
-
The register initializations should be placed above
main for better visibility and maintainability.-
The loop labels barely stand out, and this really hurts structure. The loop procedures are all crammed together and
L1 looks like just another procedure. You could at least rename L1 and L2 to Loop1 and Loop2 respectively.-
The procedures should be properly indented to help them stay isolated.
P:, for instance, hardly stands out:P: add $t1, $t1, $t5 # increment i
move $t2, $t6
bgt $t1, $t0, E
j L2All of that could should be indented together and with
P: on its own line:P:
add $t1, $t1, $t5 # increment i
move $t2, $t6
bgt $t1, $t0, E
j L2Comments
-
The side comments here seem redundant:
# Load variables into registers
lw $t1, i # $t1 = i
lw $t2, k # $t2 = k
lw $t3, p # $t3 = p
lw $t5, c # $t5 = c
lw $t6, d # $t6 = dThe top comment describes this block clearly enough, whether or not the reader already knows that
lw means load word. The side comments don't add anything else here.-
This comment is also redundant:
syscall # call operating sysUnless there are some special circumstances to note, such comments do not add anything important. Readers familiar with MIPS commands will already know what this means. You may just mention this at the top of the program if you'd like for anyone to know what this means. Putting it with every call just clutters your code.
This, on the other hand, doesn't state the obvious:
add $t2, $t2, $t5 # increment kWithout the comment, all I can tell is that this is equivalent to
$t2 += $t5, and not that it increments k. These are situations where commenting on separate commands is helpful.Overall, this is just very hard to follow. If there are bugs in here, it would be difficult to isolate them. This, in turn, hurts maintainability. Comments alone can't help with this, either. The overall structure must be sound, otherwise fixing problems is just a matter of picking apart separate lines, one-by-one.
Code Snippets
P: add $t1, $t1, $t5 # increment i
move $t2, $t6
bgt $t1, $t0, E
j L2P:
add $t1, $t1, $t5 # increment i
move $t2, $t6
bgt $t1, $t0, E
j L2# Load variables into registers
lw $t1, i # $t1 = i
lw $t2, k # $t2 = k
lw $t3, p # $t3 = p
lw $t5, c # $t5 = c
lw $t6, d # $t6 = dsyscall # call operating sysadd $t2, $t2, $t5 # increment kContext
StackExchange Code Review Q#24874, answer score: 7
Revisions (0)
No revisions yet.