7
\$\begingroup\$

I have recently started a project of SIMD libraries development for C++ on FASM for x86-64 Linux. I would be glad to hear any opinion or feedback about the project, cleanness of the code and documentation. Here is the project's web site on SourceForge.

This is just a fragment of code (addition of two vectors) and some kind of comments next to asm directives.

;==============================================================================;
;       Binary operations                                                      ;
;==============================================================================;
macro    SCALAR    op, x
{
;---[Parameters]---------------------------
array   equ     rdi                        ; pointer to array
size    equ     rsi                        ; array size (count of elements)
value   equ     xmm0                       ; value to process with
;---[Internal variables]-------------------
temp    equ     xmm1                       ; temp value
if x eq s
bytes   = 4                                ; array element size (bytes)
else
bytes   = 8                                ; array element size (bytes)
end if
step    = 16 / bytes                       ; step size (in bytes)
;------------------------------------------
        sub     size, step                 ; size -= step
        jb      .sclr                      ; if (size &lt step) then skip vector code
    clone_flt   value, x                   ; Duplicating value through the entire register
;---[Vector loop]--------------------------
@@:     movup#x temp, [array]              ; temp = array[0]
        op#p#x  temp, value                ; do operation to temp value
        movup#x [array], temp              ; array[0] = temp
        add     array, 16                  ; array++
        sub     size, step                 ; size -= step
        jae     @b                         ; do while (size >= step)
;------------------------------------------
.sclr:  add     size, step                 ; size += step
        jz      .exit                      ; If no scalar code is required, then exit
;---[Scalar loop]--------------------------
@@:     movs#x  temp, [array]              ; temp = array[0]
        op#s#x  temp, value                ; do operation to temp value
        movs#x  [array], temp              ; array[0] = temp
        add     array, bytes               ; array++
        dec     size                       ; size--
        jnz     @b                         ; do while (size != 0)
                1
;------------------------------------------
.exit:    ret
}
;:::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::::
macro    VECTOR    op, x
{
;---[Parameters]---------------------------
target  equ     rdi                        ; pointer to target array
source  equ     rsi                        ; pointer to source array
size    equ     rdx                        ; array size (count of elements)
;---[Internal variables]-------------------
value   equ     xmm0                       ; value to process with
temp    equ     xmm1                       ; temp value
if x eq s
bytes   = 4                                ; array element size (bytes)
else
bytes   = 8                                ; array element size (bytes)
end if
step    = 16 / bytes                       ; step size (in bytes)
;------------------------------------------
        sub     size, step                 ; size -= step
        jb      .sclr                      ; if (size &lt step) then skip vector code
;---[Vector loop]--------------------------
@@:     movup#x value, [source]            ; value = source[0]
        movup#x temp, [target]             ; temp = target[0]
        op#p#x  temp, value                ; do operation to temp value
        movup#x [target], temp             ; target[0] = temp
        add     source, 16                 ; source++
        add     target, 16                 ; target++
        sub     size, step                 ; size -= step
        jae     @b                         ; do while (size >= step)
;------------------------------------------
.sclr:  add     size, step                 ; size += step
        jz      .exit                      ; If no scalar code is required, then exit
;---[Scalar loop]--------------------------
@@:     movs#x  temp, [target]             ; temp = target[0]
        op#s#x  temp, [source]             ; do operation to temp value
        movs#x  [target], temp             ; target[0] = temp
        add     source, bytes              ; source++
        add     target, bytes              ; target++
        dec     size                       ; size--
        jnz     @b                         ; do while (size != 0)
;------------------------------------------
.exit:    ret
}

;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~;
;       Addition                                                               ;
;~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~;

; Scalar addition
AddS_flt32:        SCALAR    add, s
AddS_flt64:        SCALAR    add, d

; Vector addition
AddV_flt32:        VECTOR    add, s
AddV_flt64:        VECTOR    add, d

I need feedback on style of code writing and readability of comments for most programmers. Or I need cleaner style for code and its description.

Because this is a piece of program from big open source project, it should be human-readable. And I just need some kind of critique and proposition from other programmers.

\$\endgroup\$
  • \$\begingroup\$ For people, who would like to see the library speed, I made a simple performance testing page with graphics. I compare LinAsm algorithms with their GNU libc analogs. Here is the link to LinAsm performance tests. \$\endgroup\$ – Jack Black Aug 29 '12 at 20:52
5
\$\begingroup\$
array   equ     rdi                        ; pointer to array
size    equ     rsi                        ; array size (count of elements)
value   equ     xmm0                       ; value to process with

I don't particularly care for these. It seems like you're trying to make the code look more like a high-level language, but it seems to me that it ends up neither fish no fowl; it loses readability for those accustomed to assembly language without seeming to really gain much (if anything) for those accustomed to higher level languages.

temp    equ     xmm1                       ; temp value
if x eq s
bytes   = 4                                ; array element size (bytes)
else
bytes   = 8                                ; array element size (bytes)
end if
step    = 16 / bytes                       ; step size (in bytes)

I think I'd write this something more like:

chunk_size = 16
if x eq s
element_size = 4
else
element_size = 8
end if
step_size = chunk_size / element_size

;---[Vector loop]--------------------------

At least in my opinion, @@ labels should be reserved for times when the meaning is exceptionally obvious. Preceding an @@: with a comment describing its meaning indicates that you'd probably be better off with a normal label in this case.

With those, your code would come out closer to:

        sub     rsi, step_size             ; size -= step
        jb      .sclr                      ; if (size &lt step) then skip vector code
    clone_flt   xmm0, x                    ; Duplicate value through entire register

vector_loop:
        movup#x xmm1, [rdi]                ; temp = array[0]
        op#p#x  xmm1, xmm0                 ; do operation to temp value
        movup#x [rdi], xmm0                ; array[0] = temp
        add     rdi, chunk_size            ; array++
        sub     rsi, step_size             ; size -= step
        jae     vector_loop                ; do while (size >= step)
;------------------------------------------
.sclr:  add     size, step                 ; size += step
        jz      .exit                      ; If no scalar code is required, then exit
scalar_loop:
        movs#x  xmm1, [rdi]                ; temp = array[0]
        op#s#x  xmm1, xmm0                 ; do operation to temp value
        movs#x  [rdi], temp                ; array[0] = temp
        add     rdi, element_size          ; array++
        dec     rsi                        ; size--
        jnz     scalar_loop                ; do while (size != 0)

The code itself is quite nicely done, especially doing subtraction before the main loop to avoid a cmp before the jmp in the main loop. Kudos!

I can only make a couple of possible suggestions about the code itself:

  1. issuing some prefetch instructions if/when rsi is greater than a few hundred or so (but maybe it never is for your uses). Prefetching can be a little difficult to get right, and this is a simple linear pattern, so the prefetch hardware may well be perfectly adequate to the job.

  2. unrolling a few iterations of the loop--but then again, especially for simple instructions, it may be memory bound already and unrolling would just make the code bigger (and pollute more code cache) with little or no speed gain to compensate.

Neither of these has any real certainty of improvement, but if you're feeling adventurous some day, they might be worthy of a little experimentation (if you haven't already).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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