I have to find the gcd of 3 unsigned 16 bit numbers. It runs but does not give the proper output. What can be wrong in my code?

Question

This is an assembly language 8086 program to find the gcd of three unsigned 16 bit numbers.

I have checked the code several times and I feel this should be correct.

It runs and gives neither error nor warning but it gives output as different symbols (like smileys, arrows and all).

I can't figure out what to do next.

.model small
.stack 100h
.data
        arr DW 4800h,1600h,3200h
        gcd DW ?

.code
        main proc
                mov ax,@data
                mov ds,ax

                mov cx,0
                mov si,offset arr
                mov bx,[si]
                jmp L1

        L1:

                inc si
                inc cx
                mov ax,[si]

                jmp compare

       compare:

                cmp ax,bx
                jg up1
                jb excg
        up1:


                aad
                div bx
                cmp dx,0000h
                je exit
                jmp up2


       excg:
               xchg ax,bx
               jmp up1
       up2:

                mov ax,bx
                mov bx,dx
                mov dx,0
                jmp up1
       exit:
                mov gcd,bx
                cmp cx,2
                jne L1
        je display

      display:         


                mov dx,bx
                mov ah,2
                int 21h

                mov ax,4c00h
                int 21h

                main endp

     end main

Normally getting symbols and stuff means your integer->ASCII conversion is wrong, e.g. printing a single `x + '0'` byte when `x` is outside the 0-9 range, and thus needs multiple characters to represent in ASCII decimal. Use a debugger to see what's going on. — Peter Cordes, Apr 04 '19 at 07:04
It looks like your code can reach `div bx` without setting DX. So the upper half of your dividend is whatever garbage was left there. Also, `jg up1` (signed greater-than) / `jb excg` (unsigned below) definitely requires a comment!! Why do you want to jump to `excg` only if `ax signed bx)`? Also, how does [`AAD`](https://www.felixcloutier.com/x86/aad) make any sense? Your inputs are binary integers, not 2-byte unpacked BCD, I'm pretty sure. — Peter Cordes, Apr 04 '19 at 07:11
Before `mov ax,[si]` you got but a single `inc si`. Since you're dealing with words this needs to be `add si, 2`. — Fifoernik, Apr 04 '19 at 10:41
*I have checked the code several times and I feel this should be correct.* You should try to confirm your feelings by checking your calculations step by step in a debugger. — lurker, Apr 04 '19 at 11:07

score 2 · Accepted Answer · answered Apr 05 '19 at 16:51

This is a assembly language 8086 program to find the gcd of three unsigned 16 bits number.

You're trying to implement Euclid's algorithm but your code turned out to be messy and faulty.

. The pointer SI must increment by 2 since you're working with words.
. The aad instruction should really be a cwd instruction.
. The jmp compare and je display instructions are totally redundant (*).
. The single character output function is not enough to display a 16-bit number. Check "Displaying numbers with DOS" to learn how to display your result.

This is my version of your program. You'll recognize much of what you've written but in a better and correct order...

  mov   ax, @data
  mov   ds, ax
  mov   cx, 2
  mov   si, offset arr
  lodsw
  mov   bx, ax
next:
  lodsw
  cmp   bx, ax      ;We want to divide the bigger number by the smaller one
  jb    divide
  xchg  bx, ax
  jmp   divide
repeat:
  mov   ax, bx
  mov   bx, dx
divide:
  xor   dx, dx      ;Zero DX
  div   bx
  test  dx, dx      ;Test if DX is zero
  jnz   repeat
  dec   cx
  jnz   next
  ; the GCD is in BX at this point
  ; use the above link to "Displaying numbers with DOS" to show the result.

At the expense of doing 1 division extra, it is possible to avoid ordering the numbers beforehand. That 1st division will establish the correct ordering for you. Moreover that 1st division is only extra half the time (on average).
This in turn makes it easy to simplify the jumping in the code. Less jumping around is always a good thing (*).
Furthermore I've not used CX since I could just as well iterate based on SI. Why waste a good register?

  mov   ax, @data
  mov   ds, ax
  mov   si, offset arr
  mov   dx, [si]
next:
  add   si, 2
  mov   bx, [si]
repeat:
  mov   ax, bx
  mov   bx, dx
  xor   dx, dx
  div   bx
  test  dx, dx
  jnz   repeat
  mov   dx, bx
  cmp   si, offset arr+4    ;End of the array minus 2
  jb    next
  ; the GCD is in BX (and also in DX) at this point
  ; use the above link to "Displaying numbers with DOS" (it's really good!) to show the result.

Thank you very much.It really worked.Also, displaying numbers with dos was very helpful. — Natalie, Apr 06 '19 at 07:23

I have to find the gcd of 3 unsigned 16 bit numbers. It runs but does not give the proper output. What can be wrong in my code?

1 Answers1