beginner in mips, stuck on program please help!

[tpluscomb]

I've been working on a program which reads in a sequence of numbers entered by the user. It's read in as a string and then converted to numbers. Then I am suppose to get the sum of those numbers and report it back. I got most done and I know I'm close (hope!) but I am stuck at trying to get the number to sum up correctly. This is what I have so far:

.data
Prompt1:    .asciiz       "\nPlease enter a list of integers."
newline:	.asciiz		  "\n"
Buffer:		.space		  400

            .globl        main
            .text
main:                                   
		li	  	$v0, 4				# syscall for Print String
		la	  	$a0, Prompt1  		# load address of prompt into $a0
		syscall						# print the prompt

		li	  	$v0, 8		    	# syscall for Read String
		la 	  	$a0, Buffer
		li	  	$a1, 400
		syscall						# read the value of a into $v0
		la    	$s0, Buffer     	# $s0 <= Buffer

		li    	$v0, 4				# output entered string
		la	  	$a0, Buffer
		syscall

		li 		$v0, 4				# output a newline
		la 		$a0, newline
		syscall

		li 	  	$t1, 32         	# set $t1 to ' '
		move 	$s1, $zero			# set value to 0
		move	$s3, $zero			# set temp value to 0

while:
	    lb      $t2, 0($s0)       	# set p*
  		beq  	$t2, $zero, exit	# exit if p* is null
		move	$s2, $zero			# reset current character value
		addi	$s2, $t2, -48		# set current character value

		addi	$s4, $s0, 1			
		lb		$t3, 0($s4)			# set (p+1)*
		
		bne		$t3, $t1, multiply	# Go to multiply if the next character is not whitespace

		add		$s3, $s3, $t2		# temp value = temp value + character value

		add		$s1, $s1, $s3		# value = value + temp value	

		addi	$s0, $s0, 1			# increment pointer p*
		move	$s3, $zero			# reset temp value
		j		while				# go to beginning of while loop


multiply:
		
		sll		$t4, $s3, 3			# x = value multiplied by eight
		sll		$t5, $s3, 1			# y = value multiplied by two
		add		$s3, $t4, $t5		# value = x + y = value * 10

		add		$s3, $s3, $t2		# add the current character to the multiplied value

		addi	$s0, $s0, 1			# increment pointer p*

		j		while				# go to beginning of while loop


exit:
		li 		$v0, 1				# output the value for the sum
		move 	$a0, $s1 
		syscall

		li       $v0, 10           	# terminate run
        syscall                     # return to operating system

# note : the teacher said we are not allowed to use mutiplication yet because he hasn't taught us that yet so that is the cause for the shifting.

Any help is appreciated thanks!

[Duoas]

Alas, you've got a couple of serious errors...

1) Get a string syscall
When you use the systrap to get a string, the returned string has that NL character there at the end... So if I enter "1 2 3" the returned string is (in C parlance) 1 2 3\n . Your code checks for the string ending with zero (which is correct and which it does) and it checks for the space character, but it doesn't check for the newline character, which is then treated like an ascii digit (which it isn't) and converted to a value by subtracting '0' (48). Since NL is 10, 10-48 = -38, which gets added into your number.

So, you'll have to test for it.

beq	$t2,	$zero,	exit	# exit if p* is null char at end of string
	li	$t9,	'\n'		# or if p* is '\n' char at end of string
	beq	$t2,	$t9,	exit	#

As a side note, you should generally avoid using ASCII constants directly.
Use ' ' for space and '\n' for newline and '0' for the first digit character, etc.

2) Structure
This problem and the next are related, but you need to think about this above that. In other words, fixing the next problem depends on how you fix this problem.

I would have written the algorithm using two loops (one for the line of numbers, and one for each individual number). You have combined them into one loop with a branch. There is nothing wrong with doing it that way, but it is a little bit trickier to think about, because you have to think about more things at one time.

This has caused the major error: you are looking ahead to the next character in the string and only multiplying if it is not a space. However, the value of the next character has absolutely nothing to do with the value of the current character. If the current character is a digit, you'll call the multiply no matter what the next character is. Make sense?

When designing your algorithm, it is always a good idea to think of it in a high-level way. Here is what you are doing in a C pseudocode:

sum_of_numbers = 0;  /* previously named "value" */
current_number = 0;  /* previously named "temp value" */
while:
{
    current_char = *p;
    if (current_char == '\0') goto exit;

    char_value = *p - '0';

    next_char = *(p + 1);
    if (next_char != ' ') goto multiply;

no_multiply:
    current_number += current_char;  /* this is a bug */

    sum_of_numbers += current_number;  /* good */

    p++;  /* good */
    current_number = 0;  /* good */
    goto while;

multiply:
    sum_of_numbers *= 10;  /* a bug! */

    sum_of_numbers += current_char;  /* a bug! */

    p++;  /* good */
    goto while;
}
exit:

What you need to do is fix the above pseudo so that the algorithm works correctly. Remember, if the current character is a space, add the current_number to the sum_of_numbers. If the current character is not a space, then set current_number = (current_number * 10) + char_value.

3) Register usage and variable names
You may have noticed that I renamed the two of the registers that you did give names to, and I gave names to most of the other registers... and that I used the names I gave them in the pseudocode instead of the registers' actual names.

This is because (and I know you know this, but it needs repeating) the registers are your variables. Always give your variables good names.

What happens when you don't do that ahead of time is you wind-up using too many registers. I realize that the it happens because you are trying to avoid collisions, but what you need to do is go back and decide exactly what value is represented by which register, and which registers can be used without names for miscellaneous two- or three-line stuff. Write it down in a little table.

For example:

reg	name			previous

$s0	p (ptr into Buffer)	$s0	p
$s1	sum_of_numbers		$s1	value
$s2	current_number		$s3	temp value

$t0	current_char		$t2

The registers are your variables. Now, when you turn your pseudocode into assembly, you can just look-up the register from the name. I would put the register-name table as a comment in the code.

It is entirely possible that more than one name may be represented by a single register. When I wrote my version of this code, I had

$t0	current_char
$t0	char_value

This is because once it is determined that the current char is not a space, newline, or zero, then I can just turn it into the char_value and forget the current_char.

The names that you give things are very important when writing code. It clarifies the algorithm.

4) Commentary
Comments should generally describe what is happening, not how it is happening.

Bad: # exit if p* is null That's obvious from the code. A better comment would be # exit if p is at the end of the string For example:

# Exit if current_char is at the end of the input string
	beq	$t0,	$0,	done	# (current_char == '\0')
	li	$t9,	'\n'		#
	beq	$t0,	$t9,	done	# (current_char == '\n')

Bad: # set p* The problem with this line is two-fold: one, it isn't clear what you are doing (or why you are doing it in context), and two, it states that you are doing something that you are not. p* is not being modified in any way. What you are doing is getting the value of p*. A better comment would be: # current_char = p* Now, before you feel bad, you made some very good comments also...

Good: # Go to multiply if the next character is not whitespace This is very clear, easy to read, explains what you are doing. (Unfortunately you shouldn't be doing this... as noted above.)

Good: # x = value multiplied by eight # y = value multiplied by two [B]# value = x + y = value * 10[/B] (very nice!)
I like this commentary because it explains exactly what is happening and why it is being done.

Well, that's a big post. It seems like a lot but I hope I have explained myself clearly...

Hope this helps.