HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Calculating prime factors in MIPS assembly

Submitted by: @import:stackexchange-codereview··
0
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   2

Solution

Procedures

-
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 L2


All 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 L2


Comments

-
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 = d


The 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 sys


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:

add $t2, $t2, $t5   # increment k


Without 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 L2
P:  
    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 = d
syscall       # call operating sys
add $t2, $t2, $t5   # increment k

Context

StackExchange Code Review Q#24874, answer score: 7

Revisions (0)

No revisions yet.