2

So i am trying learn assembly and my practice sheet has an example where i have to create a program to input 10 numbers one at a time into an array. I have to print the highest value and when it was entered. I have barely any exp in comparing but I want to somehow store the high value and compare it the locations?

code:

include irvine32.inc
.data 
  num  dw 10 dup(0)
  count db 1
  prompt db "Enter a number: ",0
  yesMsg db "hi val is in location ",0
  hiMsg  db "The high value is ",0

.code
main proc
  mov ebx,offset num
  mov ecx,10

LOOP1:
  mov edx,offset prompt
  call writestring
  call readint
  mov [ebx],ax
  add ebx,2
  loop LOOP1

  mov ecx,10
  mov ebx,offset num
  sub eax,eax
  call crlf

LOOP2:
  mov ax,[ebx]
  call writeint
  call crlf
  add ebx,2
  loop LOOP2

  mov ebx,offset num
  mov ecx,lengthof num

FindGreatest:
  call crlf
  mov ebx,offset num
  mov ecx,lengthof num
  mov ax,[ebx]
  movsx eax,ax

FindLoop:
  cmp ax,[ebx]
  jge FindCont
  mov ax,[ebx]

FindCont:
  add ebx,2
  loop FindLoop
  mov edx,offset HiMsg
  call writestring
  call writedec
  call crlf

TestLoop:
  mov eax,ebx
  cmp [ebx],ax
  je IsHighNum

IsHighNum:
  mov edx,offset yesMsg
  call writestring
  movsx eax,count
  call writedec
  call crlf

 ENDITALL:
exit

main endp
end main

I enter 1,2,3,4,5,6,7,8,9,10

out : high value is 10 high is in location 1

Michael Petch
  • 46,082
  • 8
  • 107
  • 198
  • Remember to comment your code. Otherwise assembly code become unreadable even for the people that wrote it after some time. – Seki Dec 02 '15 at 10:04

2 Answers2

1

Your FindGreatest loop should do all the work instead of having a second loop (unfinished TestLoop) find out the position. Here's my suggestion:

To force this code to store the current position (held in EDX) into the count variable at least once I changed the jge to a jg.
By using a current position it was possible to no longer use the ECX register for the loop control.

FindGreatest:
 call  crlf
 mov   ebx, offset num
 xor   edx, edx
 mov   ax, [ebx]
FindLoop:
 inc   edx          ;Positions 1-10
 cmp   ax, [ebx]
 jg    FindCont
 mov   ax, [ebx]
 mov   count, edx   ;Remember position of greatest
FindCont:
 add   ebx, 2
 cmp   edx, lengthof num
 jb    FindLoop

Now you're ready to output the result in AX and the position in count.
Remember to sign-extend AX using movsx eax, ax. You could not have done this before the loop because if the first number were negative and the rest of the array contained at least one positive number then the output would have been wrong!

Sep Roland
  • 33,889
  • 7
  • 43
  • 76
0

First there are some readability / performance improvements:

Replace

mov ax,[ebx]
movsx eax,ax

with

movsx  eax, word [ebx]

And in this code:

FindLoop:
  cmp ax,[ebx]
  jge FindCont
  mov ax,[ebx]
FindCont:
  add ebx,2
  loop FindLoop     ; leave an empty line after the loop, not in the middle of the loop and then no space after.

  mov edx,offset HiMsg

In your loop, you're already set up to do more things when you find out that you've found a new max. e.g. save ebx into another reg, like edx. Do this with another mov instruction next to the mov ax,[ebx], so the jge will skip it, too.

At the end of the loop, you can subtract pointers and right-shift (to divide by two) to find the index where the max element was.

Or instead of storing the pointer, you could store the current value of ecx. You're using a loop-counter and a pointer increment. The loop instruction is slow, and not recommended. Your code would be better if you compared ebx to see if it's past the end of the array yet, as a loop condition.

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
  • I am unclear on the subtract pointer and right shift ... again im very new – Anthony Iarossi Dec 02 '15 at 23:50
  • @AnthonyIarossi: If you have two pointers into an array of 2-byte elements, the difference in index is `(b-a)/2`. You can and should implement unsigned division by two with a right-shift. Use `b=position_where_max_is`, and `a=start_of_array`. – Peter Cordes Dec 03 '15 at 00:32
  • Another way altogether would be to use an indexed addressing mode. Scan backwards from the end of the array, since counting down in asm is more efficient than counting up. (`dec ecx` sets ZF when it reaches zero, and `SF` when it goes negative. So it's like `do{ m=max(m,arr[ecx]); }while(--ecx >= 0);` instead of `do{ m=max(m,*arr++); } while(--ecx>0);` – Peter Cordes Dec 03 '15 at 00:39
  • @AnthonyIarossi: Your current loop could do `cmp ax, [ebx + ecx*2 - 2]`. (The -2 is because you want to loop from n-1 .. 0, not from n to 1.) Finding the max works whether you scan forwards or backwards. But what I was saying about looping with `dec ecx / jnc` (instead of `loop`) was that you could drop the `-2` from the address that way, because then ecx would go from n-1 to 0 (and be -1 after the last dec and not-taken branch). CF will be set when you decrement 0 and it wraps around to UINT_MAX. – Peter Cordes Dec 03 '15 at 01:26
  • look at some examples of real code, or disassembly. There are lots of SO posts with commented ASM. e.g. Michael Petch wrote [a nice simple loop](http://stackoverflow.com/a/34048177/224132) to store each bit of a register as an ASCII `'0'` or `'1'` in an array, to help another asm beginner earlier today: – Peter Cordes Dec 03 '15 at 01:30