Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is there any way to make this more efficient?

.386            ; assembler use 80386 instructions
.MODEL  FLAT    ; use modern standard memory model 

INCLUDE io.h    ; header file for input/output

cr      EQU     0dh     ; carriage return
Lf      EQU     0ah     ; line feed
maxArr  EQU     5       ; constant for array size

ExitProcess PROTO NEAR32 stdcall, dwExitCode:DWORD
EXTERN      SEARCH:near32

.STACK  4096    ; reserve 4096-byte stack

.DATA   ; reserve storage for data


prompt0     BYTE    cr, Lf, 'Please enter 5 numbers.', cr, Lf
            BYTE    'This program will then search the array for a specific '
            BYTE    'value: ', 0

array       DWORD   maxArr DUP (?)  ; array variable, size: maxArr
elemCount   DWORD   ?               ; number of elements entered
valToSearch DWORD   ?               ; value to search for

prompt1     BYTE    cr, Lf, 'Which value would you like to search for?: ', 0
dwinput     BYTE    16 DUP (?)      ; for input

poslabel    BYTE     cr,Lf,Lf, 'The value was found at position (0 if not found): '
dwoutput    BYTE    16 DUP (?), cr, Lf, 0   ; for output

noPos       BYTE    cr, Lf, 'The value entered is not present in the array.', 0

.CODE   ; program code

_start:     ; program entry point

    output  prompt0         ; output directions and prompt input
    mov     ecx, maxArr     ; initialize ECX with the array capacity value
    lea     ebx, array      ; place address of array in EBX
    xor     edx, edx        ; initialize EDX

getArrayInput:
    input   dwinput, 16     ; get input
    atod    dwinput         ; convert to DWORD, place in EAX

    jo      subroutine          ; if any overflow, end number entry

    mov     [ebx], eax      ; store number in address pointed to by EBX (array                              ; index position)
    inc     edx             ; increment counter if number entered so far
    add     ebx,4           ; get address of next item of array (4 bytes away)
    loop    getArrayInput   ; loop back (up to 5 times)


subroutine:
    output  prompt1         ; get value to search for
    input   dwinput, 16     ; get input
    atod    dwinput         ; convert to DWORD, place in EAX
    mov     valToSearch, eax    ; store value to search for

    mov     elemCount, edx  ; move no. of elements to elemCount 
    lea     eax, array      ; get starting address of array again
    push    eax             ; Parameter 1: push address of array (4 bytes)
    push    elemCount       ; Parameter 2: push elemCount by value (4 bytes)    
    push    valToSearch     ; Parameter 3: push address of valToSearch (4 bytes)

    call   SEARCH           ; search for value, return eax

    add esp, 12             ; remove arguments from stack

    dtoa    dwoutput, eax   ; convert to ASCII
    cmp     eax, 0          ; check if eax(position) = 0
    je      zeroPosition    ; if position=0, go to error message
    output  poslabel        ; output the position
    jmp     exitSeq         ; exit the program

zeroPosition:   
    output  noPos   ; output error & exit

exitSeq:

INVOKE ExitProcess, 0   ; exit with return code 0
PUBLIC _start           
END            



SUBROUTINE:--------------------------------------------------



.386            ; assembler use 80386 instructions
.MODEL  FLAT    ; use modern standard memory model 

PUBLIC SEARCH       ; make SEARCH proc visible

.CODE   ; program code

SEARCH  PROC   NEAR32

    push    ebp         ; save base pointer
    mov     ebp,esp     ; establish stack frame
    push    ebx         ; save registers
    push    ecx
    push    edx
    pushf               ; save flags

    mov     eax, [ebp+8]    ; move value to search for to eax
    mov ebx, [ebp+16]       ; move array address to EBX
    mov ecx, [ebx]      ;move first element to ECX
    cmp ecx, eax        ;comparing search number to the first value in the array
    je first                ;If equal return the position.

    mov ecx, [ebx+4]        ;move first element to ECX
    cmp ecx, eax        ;comparing search number to the second value in the array
    je second           ;If equal return the position.

    mov ecx, [ebx+8]
    cmp ecx, eax    ;comparing search number to the third value in the array
    je third                ;If equal return the position.

    mov ecx, [ebx+12]
    cmp ecx, eax        ;comparing search number to the fourth value in the array
    je fourth               ;If equal return the position.

    mov ecx, [ebx+16]
    cmp ecx, eax        ;comparing search number to the fifth value in the array
    je fifth                ;If equal return the position.
    jmp none

first:                  ;returns position 1
    mov eax, 1      
    jmp done

second:             ;returns position 2
    mov eax, 2
    jmp done

third:                  ;returns position 3
    mov eax, 3
    jmp done

fourth:             ;returns position 4
    mov eax, 4
    jmp done

fifth:                  ;returns position 5
    mov eax, 5
    jmp done

none:                   ;returns 0 if the search value is not found.
    mov eax, 0
    jmp done

done:


retpop: 
    popf                ; restore flags
    pop     edx         ; restore registers
    pop     ecx 
    pop     ebx          
    pop     ebp         ; restore base pointer
    ret                 ; return to main

SEARCH  ENDP

PUBLIC SEARCH

END      
share|improve this question

2 Answers 2

There are some opcodes I haven't seen before: output, input, and atod; are these new opcodes, macros, or what?

"if any overflow, end number entry" might not be correct behaviour: perhaps you should prompt again for correct input, or abend the program.

I don't know whether "loop getArrayInput" will work because I don't know whether input and atod will preserve the contents of the ecx register.

"add esp, 12" implies that SEARCH is using __cdecl calling convention, in which case you could name it _SEARCH with an underscore. Alternatively SEARCH could pop its own parameters using ret 12 instead of ret.

There is a way to make the search more "efficient": use the scasd opcode with the repnz prefix, i.e. repnz scasd. That would be many fewer instructions. It would be faster too, on '386 processors (I don't know about Pentium+ processors which prefer more, RISC-like opcodes).

share|improve this answer

It's been a while since my assembler days but your SEARCH routine should really use a loop to check the array. Basically it should accept the start address of the array, the number of entries and the number to search for. Right now if the requirement changes to lets say have an array of 10 numbers you are in to do a lot of copy and paste and bound to make mistakes. You have already used a loop for the input so why not for the search as well?

Also some of the names like prompt0 could be a tiny bit more descriptive.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.