Assembly newbie requesting help: signed integer division + "truncating to an integer"

Question

I asked a question regarding this code several days ago, so some of you may find it familiar. For those of you who do not find it familiar, what this code is supposed to do is request 25 signed integers from the user and store them into an array; this is done via the requestSignedInts subroutine (I'm pretty sure this part is working correctly). Afterwards, a "calcMean" subroutine is supposed to add up all of the values in the array, divide them by the number of elements in the array, and then "truncate the result to an integer". This is where I'm stuck. I attempted to write a calcMean subroutine that would do what I described just now, but cannot seem to figure out how to do the division properly. On top of that, I'm not even sure if what I currently have in my calcMean subroutine would work properly. Can anyone provide assistance?

INCLUDE    c:\irvine\irvine32.inc
INCLUDELIB c:\irvine\irvine32.lib
INCLUDELIB c:\masm32\lib\user32.lib
INCLUDELIB c:\masm32\lib\kernel32.lib

.data
theSINTArray BYTE 25 dup(?)
lengthOfArray BYTE ?
indexCounter BYTE 0              
prompt BYTE "Please enter a value: ",0


.CODE

main PROC

    call    requestSignedInts
    call    calculateMean
    exit

main ENDP

requestSignedInts PROC

    mov     edx, offset theSINTArray
Next:
    push    edx                                        
    mov     edx,OFFSET prompt                          
    call    WriteString                                
    call    ReadInt
    pop     edx 
    mov     [edx], al
    inc     edx
    cmp     edx, offset theSINTArray + 25
    jb      Next
    ret

requestSignedInts ENDP


calculateMean PROC

push ecx
mov    ecx,lengthOfArray - theSINTArray ; Determine array length    
xor    eax, eax                         ; Clear EAX
mov    esi, OFFSET theSINTArray         ; Starting point for index into array
calcMean:
movsx  edx, byte ptr[esi]               ; Sign extended move a byte into EDX
add    eax, edx                         ; Accumulate in EAX
inc    esi                              ; Increment source pointer to the next element
loop   calcMean                         ; or cmp esi,endOfArray / jb, then you wouldn't need to touch ecx

mov    ecx,lengthOfArray - theSINTArray ; Determine array length    
cdq                                     ; sign-extend eax into edx:eax
idiv   ecx                              ; Divide edx:eax by ecx
                                        ; eax now contains the integer and edx contains
                                        ; the remainder.
pop    ecx
ret

calculateMean ENDP

END     main

Division already truncates to an integer, so just do nothing. btw remember the role of `edx` in division. — harold, Apr 12 '16 at 22:17
The "idiv" instruction gives integer result. As @harold says, just do nothing, the result is already truncated in EAX (http://www.cs.virginia.edu/~evans/cs216/guides/x86.html). — Jose Manuel Abarca Rodríguez, Apr 12 '16 at 22:21
@harold If there is a "role" for edx, I unfortunately do not know what it is. I'm assuming that this "role" is expected to be common sense, but as a newbie to the language, I don't have all the basics down yet. What exactly is the role of edx? — Proto, Apr 12 '16 at 22:22
Search "idiv" in the link http://www.cs.virginia.edu/~evans/cs216/guides/x86.html, EDX is the remainder of the division while EAX is the quotient, you only want the quotient, right? — Jose Manuel Abarca Rodríguez, Apr 12 '16 at 22:23
@JoseManuelAbarcaRodríguez I did as you suggested, and if I understand correctly, the line that I have in my code: "idiv eax", divides edx:eax by eax. If that is correct, I have two more questions. 1. Should I be using a different register to store the sum of all of the elements in the array? 2. How would I go about placing the most significant four bytes of the sum into edx and the least significant 4 bytes into eax? — Proto, Apr 12 '16 at 22:30

score 2 · Accepted Answer · edited Apr 13 '16 at 11:54

2

It seems to me that you might be missing the function of idiv as brought out in the comments. Why not:

calculateMean PROC

push ecx
mov    ecx,lengthOfArray - theSINTArray ; Determine array length    
xor    eax, eax                         ; Clear EAX
mov    esi, theSINTArray                ; Starting point for index into array
calcMean:
movsx  edx, byte ptr[esi]               ; Sign extended move a byte into EDX
add    eax, edx                         ; Accumulate in EAX
inc    esi                              ; Increment source pointer to the next element
loop   calcMean                         ; or cmp esi,endOfArray / jb, then you wouldn't need to touch ecx

mov    ecx,lengthOfArray - theSINTArray ; Determine array length    
cdq                                     ; sign-extend eax into edx:eax
idiv   ecx                              ; Divide edx:eax by ecx
                                        ; eax now contains the integer and edx contains
                                        ; the remainder.
pop    ecx
ret

calculateMean ENDP

You already know the length of the list, you simply need to clear edx and divide by ecx. This also properly uses a movsx to do a sign extended move of a byte (from your array) into a 32 bit register (edx). The total is accumulated into eax and, finally, we sign extend and divide.

edited Apr 13 '16 at 11:54

Peter Cordes

328,167
45
605
847

answered Apr 13 '16 at 01:08

David Hoelzer

15,862
4
48
67

1

If `eax` can be negative, you need to sign-extend `eax` into `edx:eax` with a `cdq` instruction. Use either `xor edx,edx` / `div`, or `cdq` / `idiv`. – Peter Cordes Apr 13 '16 at 01:52
Absolutely true. It felt as though OP wasn't getting how idiv might work in his code. I'll add the cdq in the morning when I'm at my keyboard. – David Hoelzer Apr 13 '16 at 02:06
There's a lot more to be said about how the OP needs to load bytes with `movsx` before accumulating into a 4B register, and that `[edx + indexCounter]` uses the address of indexCounter, not its value. I made the `cdq` edit but I'll leave the rest for you... – Peter Cordes Apr 13 '16 at 02:50
Thanks @PeterCordes. I was so focused on the `idiv` that I honestly didn't notice the array indexing silliness. – David Hoelzer Apr 13 '16 at 10:35
I ended up making a new answer, because there was so much still bad in the OP's code (e.g. ABI violations like clobbering esi but save/restoring ecx). – Peter Cordes Apr 13 '16 at 12:29
I put this solution into my code and tried to assemble, but got an assembly error with the line: "mov esi, theSINTArray". What is the reason for this? – Proto Apr 15 '16 at 19:45
Is that the correct name of the label? I was working off of what you posted. – David Hoelzer Apr 15 '16 at 21:14
Yes, it is indeed the correct label. The exact error is A2070 - invalid instruction operands. – Proto Apr 15 '16 at 21:18
Ah, could be the assembler flavor. Try `mov esi, offset theSINTArray` – David Hoelzer Apr 15 '16 at 22:11
I tried that instead and it assembled successfuly. I'll test the program now. – Proto Apr 15 '16 at 22:19
After my testing, it seems as though the program is working correctly for the most part. However, there is one strange behavior exhibited by it. When I enter 127 for all 25 of the input numbers, the mean number displayed in EAX is correct. If I do the same with any number higher than 127, the mean displayed in EAX is extremely off. What is the reason for this? – Proto Apr 15 '16 at 22:53
@Proto : 127 is the highest signed number that can fit in one byte/8bits (-128 to +127) – Michael Petch Apr 15 '16 at 23:24

score 1 · Answer 2 · edited May 23 '17 at 11:50

There's more that's weird about your original code than David's answer addressed. I'm posting a new answer instead of further editing David's.

Changes are noted in comments. Old comments removed so the change-notes stand out.

.data
SBYTEArray BYTE 25 dup(?)     ; SINT = signed int, which makes most x86 C programmers think 32bit
; lengthOfArray BYTE ?        ; totally bogus: named wrong, and there doesn't need to be any storage there
; SBYTEArray_len equ $ - SBYTEArray  ; that's NASM syntax and IDK the MASM syntax
SBYTEArray_end:                  ; SBYTEArray_end and prompt have the same address.  That's fine.

prompt BYTE "Please enter a value: ",0      ; If MASM/Windows has a .rodata section, you should put constant data there.  It goes in the text section, along with code, because each instance of your process doesn't need a private copy.


.CODE
main PROC
    call    requestSignedInts
    call    calculateMean
    exit
main ENDP

requestSignedInts PROC
    ; indent code one level deeper than labels / directives
    push    esi                     ; save a call-preserved reg
    mov     esi, offset SBYTEArray

Next:
    mov     edx,OFFSET prompt
    call    WriteString
    call    ReadInt
    mov     [esi], al
    inc     esi
    ; cmp     edx, offset SBYTEArray + 25  ; hard-coding the size defeats the purpose of defining lengthOfArray
    cmp     esi, offset SBYTEArray_end
    jb      Next

    pop     esi          ; note that the push/pop are outside the loop
    ret
requestSignedInts ENDP


calculateMean PROC
    ; push ecx          ; The normal function-call ABIs allow clobbering ecx, and I don't see any reason to make this function go beyond the ABI requirements (although that is an option in asm)
    ; push   esi          ; esi *is* call-preserved in the standard 32bit ABIs.  But by changing our function to use fewer registers, we can avoid the save/restore

    xor    eax, eax                         ; start with sum=0
    mov    ecx, offset SBYTEArray

calcMean:
    movsx  edx, byte ptr[ecx]
    add    eax, edx
    inc    ecx
    cmp    ecx, offset SBYTEArray_end
    jb     calcMean                         ; loop while our pointer is below the end pointer

    mov    ecx, SBYTEArray_end - SBYTEArray ; Determine array length.  Does this need OFFSET?
    cdq
    idiv   ecx
    ; pop esi         ; we ended up not needing it
    ret
calculateMean ENDP

END Main

loop is slow, avoid it. Esp. when you can save a register by using something else for your loop condition.

Assembly newbie requesting help: signed integer division + "truncating to an integer"

2 Answers2