Contrast reduction - intel x86

Question

I was supposed to do a project to pass my course. I would like to ask, if there is any possibility to make my code more effective or just better. I'm doing it because my coordinator is a very meticulous perfectionist and crazy about efficiency. It's a hybrid program, it modifies a 24bpp bitmap. It's a contrast reduction, algorithm looks like this(it's approved by my coordinator):

comp-=128;
comp*=rfactor
comp/=128
comp+=128

'comp' means every component of a pixel, literally: every value of red, green and blue in every pixel. The function does just this, I read from file using another functions in C. I forward to assembly an array with components, the width of the bmp, amount of pixels in each line, and the 'rfactor' - value of contrast reduction. then I just make this:

;  void contrast(void *img, int width, int lineWidth, int rfactor);
;   stack:  EBP+8 ->        *img
;       EBP+12 ->       width [px]
;       EBP+16 ->       lineWidth [B]
;       EBP+20 ->       rfactor (values in range of 1-128)
section .text 
global  contrast

contrast:
push    ebp         
mov ebp, esp     

push    ebx         
mov     ebx, [ebp+12]   ; width
mov     eax, [ebp+16]   ; lineWidth
mul     ebx     ; how much pixels to reduce
mov     ecx, eax    ; set counter
mov     edx, [ebp+8]    ; edx = pointer at img
mov     ebx, [ebp+20]   ; ebx=rfactor

loop:
xor     eax, eax    
dec     ecx         ; decrement counter
mov     al, [edx]   ; current pixel to al
add     eax, -128   
imul    bl          ; pixel*rfactor
sar     eax, 7      ; pixel/128
add     eax, 128    
mov     byte[edx], al   ; put the pixel back
inc     edx         ; next pixel
test    ecx, ecx    ; is counter 0?
jnz     loop        

koniec:
pop     ebx
mov     esp, ebp    
pop     ebp
ret 

Is there anything to improve? Thank you for all suggestions, I have to impress my coordinator ;)

Answer 1

I you are still interested in a SIMD version here is one.
It use AVX2 instructions so you need at least a 4th generation processor (Haswell micro-architecture).

BITS 32

GLOBAL _contrast

SECTION .code

;rfactor
;lineWidth 
;width
;ptr to buffer
_contrast:
    push ebp         
    mov ebp, esp     

    and esp, 0fffffff0h

    push edi
    push esi
    push ebx 
    push eax


    mov eax, DWORD [ebp+0ch]        ;witdh
    mul DWORD [ebp+10h]             ;total bytes
    mov ecx, eax                    ;Number of bytes to process
    shr ecx, 04h                    ;Process chunks of 16 bytes per cycle

    mov edi, DWORD [ebp+08h]        ;Buffer


    ;--- Prepare ymm registers ---

    vzeroall

    sub esp, 10h 

    ;ymm1 contains the r factor (x16)
    movzx ebx, WORD [ebp+14h]
    mov DWORD [esp], ebx
    vpbroadcastw ymm1, WORD [esp]   ;ymm1 = r (x16)

    ;ymm0 contains the 128-r value (x16)
    neg WORD [esp]                  ;-r
    mov al, 128
    add WORD [esp], ax              ;128-r
    vpbroadcastw ymm0, WORD [esp]   ;ymm0 = 128-r (x16)

    add esp, 10h

.loop:
    ;Computer channels values
    vpmovzxbw ymm2, [edi]           ;16 channels (128 bit) to 16 words
    vpmullw ymm2, ymm2, ymm1        ;ymm2 = in*r
    vpsrlw ymm2, ymm2, 7            ;ymm2 = in*r>>7
    vpaddw ymm2, ymm2, ymm0         ;ymm2 = in*r>>7 + r-128

    vpackuswb ymm2, ymm2, ymm2      ;xmm2 = 16 computes values 

    ;Store to memory
    movdqa [edi], xmm2

    add edi, 10h

loop .loop

    pop eax
    pop ebx
    pop esi
    pop edi 

    mov esp, ebp
    pop ebp
    ret 

I have tested it by comparing its output with the output of your code.

The prototype in C is your old one (with lineWidth):

void contrast(void* buffer, unsigned int width, unsigned int Bpp, unsigned short rfactor);

I have done some profiling on my machine. I have run this version and the one in your answer on a 2048x20480 image (120MiB buffer) 10 times. Your code takes 2.93 seconds, this one 1.09 seconds. Though this timings may not be very accurate.

This version require a buffer size that is a multiple of 16 (because it processes 16 bytes per cycle, 5 and one third of pixel at a time), you can pad with zeros. If the buffer is aligned on 16 byte boundaries it will run faster.

If you want a more detailed answer (with useful comments for example :D) just ask in the comments.

EDIT: Updated the code with the great help of Peter Cordes, for future reference.

Answer 2

Finally, I managed. Thank for all your suggestions. I hope my coordinator will be satisfied. If not, I would be forced to do it using vectors
; void contrast(void *img, int pixels, int rfactor); ; stack: EBP+8 -> *img ; EBP+12 -> counter (how much bytes to modify) ; EBP+16 -> rfactor section .text global contrast

contrast:
push    ebp 
mov     ebp, esp     
push    ebx 
push    edi

mov     ecx, [ebp+12]   ; set counter
mov ebx, [ebp+16]   ; rfactor to ebx
mov edi, [ebp+8]    ; img pointer
mov dl, 128     
sub dl, bl      ; 128-rfactor

loop:
mov al, [edi]  ; current pixel to al
mul bl      ;
shr ax, 7   ; byte*rfactor>>7+(128-rfactor)
add al, dl  ;
stosb           ; store al, inc edi
loop loop   

koniec: 
pop     edi
pop     ebx
mov     esp, ebp    
pop     ebp
ret 

Answer 3

You may squeeze a little more out of your loop when not up-counting, but downcounting, like this (I changed the header to indicate the meaning, and I omitted the trailer:

    mov edx, img
    mov ecx, width*lineWidth  ; pseudo-assembler here
    mov ebx, rfactor
loop:
        xor  eax, eax
        mov  al, [ecx+edx-1]
        sub  eax, 128
        imul bl
        sar  eax, 7
        add  eax, 128
        mov  byte ptr[ecx+edx-1], al
        dec  ecx
        jnz  loop
; add trailer here