2

I'd like to write a simple program to find the minimum value in an array. I'm using Intel 8086 architecture (if I'm right?). The problem is that I am totally new to Assembly language and well, I just cannot figure out what am I doing wrong in my code, I'm stuck.

At the end (before exiting) my ax register does not contain my result.
I'm afraid it doesn't even place it there (I check registers' values with Turbo Debugger tool).

.MODEL TINY

Data SEGMENT
                        arr_len     EQU 5
                        arr         DB 07h, 02h, 03h, 10h, 12h
                        minimum     DB 255
Data ENDS

Code SEGMENT
                        ORG 100h
                        ASSUME CS:Code, DS:Data, SS:Code
Start:
                        mov ax, SEG Data
                        mov ds, ax  ;load data to ds reg    
                        mov cx, arr_len ;arr_length to counter
                        mov bx, OFFSET arr ;load first elem.
Search: 
                        inc bx
                        cmp bl, [minimum] ;compare loaded elem with min
                        jb Swap ;if bl<minimum
Swap:
                        mov minimum, bl
                        loop Search         

                        mov al, minimum ;result here?
                        mov ax, 4C00h
                        int 21h

Code ENDS
END Start

Could anyone give me advices what is wrong here? Thanks in advance.

mtszkw
  • 2,717
  • 2
  • 17
  • 31
  • 1
    You always swap, even if the current element isn't the minimum. – user786653 Mar 26 '16 at 12:38
  • @user786653, I see, I replaced `loop` instruction to be right below `jb`. Now it should `Swap` only when I found lesser value, right? Or I still don't get it tho... – mtszkw Mar 26 '16 at 12:42
  • 1
    `CMP BL, [minimum]` compares the value in BL, not the value in arr that you want. You never load any value from the array, that is why you don't get the result you want. Rather put AL to 255, compare value of [BX] to AL and go from there. – Sami Kuhmonen Mar 26 '16 at 12:57

2 Answers2

4
  • The tiny model doesn't need setting DS explicitly. (CS=DS=ES=SS)
  • Since BX points at your array the first time, you should not immediately increment it! Do inc before looping only.
  • You didn't really compare an array element but rather the low byte of the address by which you iterate the array.
  • There's no sense in putting any result in AL if directly afterwards you exit by putting 4C00h in AX. Viewing AL using a debugger could work though.

Here's a version that can work:

Search: 
    mov al, [bx]
    cmp al, [minimum] ;compare loaded elem with min
    jnb NoSwap        ;if al>=minimum
    mov [minimum], al ;New minimum
NoSwap:
    inc bx
    loop Search
Fifoernik
  • 9,779
  • 1
  • 21
  • 27
  • 2
    I have a suspicion he intended to return the minimum value as the int 21/ah=4ch return code (which could be checked in a DOS batch file). I think `mov ax, 4C00h` should have been `mov ah, 4Ch` for what I think the expected behavior was suppose to be. – Michael Petch Mar 26 '16 at 17:17
  • Thank you so much, I do understand what I did wrong, that's so obvious. I also decided to store my minimum values in `dl` register instead of `al` so it can be clearly visible using a debugger. And it works atm. I appreciate! – mtszkw Mar 27 '16 at 11:09
2

Like Fifoernik's answer explained, your conditional branch jumps to the same place whether it's taken or not, because the branch target is the next insn.

Here's how to do it without keeping your minimum in memory as you accumulate it, because that's horrible.

This version accumulates minimum in al, never storing it to memory. I also used string ops for fun. You'll get smaller code size, but maybe slower code than loading with mov. Using si as a source pointer is a good convention to help humans keep track of things, even when not using string ops. Don't make the code worse just for that, of course, but I recommend it when you have the choice.

I also avoid loop because it's slow and wasn't gaining us much of anything. Unless you're going hard-core optimizing for code-size over speed, don't use loop.

.MODEL TINY
Data SEGMENT
    arr         DB 07h, 02h, 03h, 10h, 12h
    arr_len     EQU $-arr                   ; let the assembler calculate the size to avoid mistakes
Data ENDS

Code SEGMENT
    ORG 100h
    ASSUME CS:Code, DS:Data, SS:Code
unsigned_min:
    ;; segment setup not needed with tiny model, according to Fifoernik
    ; assume arr_len >= 2
    mov    si, OFFSET arr       ; src pointer = &first element
    lea    di, [si + arr_len]   ; end pointer.
    ;; mov di, arr_len + OFFSET arr ; also works, since input pointer is static
    ;; or use arr_len + OFFSET arr as an immediate operand for the loop condition, if you don't care about keeping the hard-coded input address out of the loop itself.

    cld                         ; clear the direction flag so string insns count upwards.  Omit if the ABI guarantees this state already
    lodsb                       ; al = min so far = first element, and advance si to point to the second element


;; on entry: AL = 1st element (min).  SI = pointer to 2nd element.  DI = end-pointer
Search:
    cmpsb                       ; compare [si] with current min (al), and ++si
    jae   .no_new_min

    mov    al, [si-1]           ; conditionally skipped.  You could use `cmov` instead of the branch on a CPU supporting 686 insns
.no_new_min
    cmp    si, di               ; loop while si < end
    jb    Search

    ; min is in AL

    mov ah, 4Ch            ; exit with AL as exit status
    int 21h

If you can't assume arr_len >= 2, then initialize min to 255 or the first element, and enter the loop with it pointing to the first element, instead of 2nd. Or use an extra cmp si,di /jb outside the loop.

Community
  • 1
  • 1
Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
  • 1
    `mov ah, 4Ch` `int 21h` is the DOS interrupt for exiting a program. _AL_ happens to be the return value that will be returned back to _DOS_. I'd just eliminate your final comment `;; make an exit syscall, or ret if this is a function` since your code has done that. `RET` would have worked too since COM programs have 0x0000 pushed on the stack at program start. PSP:0x0000 contains an `int 20h` instruction (CP/M compatibility) which will exit the program. Had this been a DOS EXE program, `ret` wouldn't work (int 21h/ah=4ch is preferred). – Michael Petch Mar 26 '16 at 16:54
  • One can tell this is a COM program because of `org 100h`, AND the `.MODEL TINY`. In this case CS=DS=SS. The first 100h bytes contain the _PSP_ (program segment prefix). COM program entry point is always at 100h from the beginning of the segment that DOS loaded the COM program into. – Michael Petch Mar 26 '16 at 17:05
  • Of course had you used `ret` in this case you can't actually return a value with that method, so I would have recommended just going with `mov ah, 4Ch` `int 21h`, where AL(return value)=minimum – Michael Petch Mar 26 '16 at 17:11
  • @MichaelPetch: thanks for clearing that up. The DOS syscall part is pretty much completely uninteresting to me, so I didn't even bother looking up what that uncommented syscall was. I assumed it was some kind of print-int. I've said many times that DOS and 16bit in general is obsolete and not worth learning. I wish I wasn't wasting brainpower on knowing the legal 16bit addressing modes, but people are always asking 16bit questions here on SO. – Peter Cordes Mar 26 '16 at 17:32
  • Don't get me wrong, I still upvoted the answer. I was just letting you know about the whole Int21/ah=4ch (or ret) convention. It appeared you may not have known that Int21/ah=4ch was the DOS exit mechanism. No worries. – Michael Petch Mar 26 '16 at 17:34
  • @MichaelPetch: yup, that was exactly the issue, and I was thanking you for taking care of the part of the answer that I didn't care enough about to get right until you commented :). And I figured you were the upvoter. – Peter Cordes Mar 26 '16 at 17:38
  • Hi guys. Thanks for you advices. I am using DOSBox emulator for this program, that's why `int21/ah=4ch` is there, that's how I was taught to do. – mtszkw Mar 27 '16 at 11:11
  • 1
    @MateuszKwasniak: Yup, I know DOS uses `int 21h` for system calls, with the call number in `AH`. You're doing it correctly. It's just that DOS is obsolete and not worth learning unless someone is forcing you to do so, which is why I was only interested in the loop part of the program, not the system call at the end. – Peter Cordes Mar 27 '16 at 11:13
  • @PeterCordes, ye, I see. Thank you! – mtszkw Mar 27 '16 at 12:12