Improve codegen for getindex(::Array, ::Face)

[sjkelly]

#28 brought this method to my attention so I made some tweaks for codegen.

Apologies for my aversion to regular macro style. I really can't figure out hygiene and have gotten perhaps too good at avoiding it.

Open questions:

• Should we make unsafe_getindex the default?

I have tested some packages depending on this and they don't seem to care that the return type changed to tuple. (we will need to tag v0.2.0 after this though).

CC: @SimonDanisch @dreammaker

Before

julia> @code_native getindex([1,2,3,4],f)
    .text
Filename: /home/steve/.julia/v0.4/GeometryTypes/src/faces.jl
Source line: 2
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 2
    pushq   %r15
    pushq   %r14
    pushq   %r12
    pushq   %rbx
    subq    $48, %rsp
    movq    $8, -80(%rbp)
    movabsq $jl_pgcstack, %r14
    movq    (%r14), %rax
    movq    %rax, -72(%rbp)
    leaq    -80(%rbp), %rax
    movq    %rax, (%r14)
    vxorps  %xmm0, %xmm0, %xmm0
    vmovups %xmm0, -48(%rbp)
    vmovups %xmm0, -64(%rbp)
Source line: 2
    movq    (%rsi), %r15
    movq    8(%rsi), %r12
    movq    16(%rsi), %rbx
    movq    %rdi, -64(%rbp)
    movabsq $jl_gc_allocobj, %rax
    movabsq $139678908196536, %rcx  # imm = 0x7F0987B4A6B8
    movabsq $139678905658840, %rdx  # imm = 0x7F09878DEDD8
    movq    (%rdx), %rdx
    movq    %rdx, -56(%rbp)
    movq    (%rcx), %rcx
    movq    %rcx, -48(%rbp)
    movl    $24, %edi
    callq   *%rax
    movabsq $139678934228976, %rdx  # imm = 0x7F098941DFF0
Source line: 2
    leaq    -56(%rbp), %rsi
Source line: 2
    movabsq $jl_f_apply, %rcx
    movq    %rdx, -8(%rax)
    movq    %rbx, 16(%rax)
    movq    %r12, 8(%rax)
    movq    %r15, (%rax)
    movq    %rax, -40(%rbp)
    xorl    %edi, %edi
    movl    $3, %edx
    callq   *%rcx
Source line: 2
    leaq    -64(%rbp), %rsi
Source line: 2
    movabsq $jl_apply_generic, %rcx
    movq    %rax, -56(%rbp)
    movabsq $139678900639408, %rdi  # imm = 0x7F09874156B0
    movl    $2, %edx
    callq   *%rcx
    movq    -72(%rbp), %rcx
    movq    %rcx, (%r14)
    addq    $48, %rsp
    popq    %rbx
    popq    %r12
    popq    %r14
    popq    %r15
    popq    %rbp
    ret

Bounds checking

julia> @code_native getindex([1,2,3,4],f)
    .text
Filename: /home/steve/.julia/v0.4/GeometryTypes/src/faces.jl
Source line: 7
    pushq   %rbp
    movq    %rsp, %rbp
    movq    %rsi, %r9
Source line: 7
    movq    8(%r9), %rsi
    movq    (%rdx), %r8
    leaq    -1(%r8), %rcx
    cmpq    %rsi, %rcx
    jae L98
    movq    8(%rdx), %rcx
    leaq    -1(%rcx), %rax
    cmpq    %rsi, %rax
    jae L117
    movq    16(%rdx), %rdx
    leaq    -1(%rdx), %rax
    cmpq    %rsi, %rax
    jae L136
    movq    (%r9), %rsi
    movq    -8(%rsi,%r8,8), %rax
    movq    -8(%rsi,%rcx,8), %rcx
    movq    -8(%rsi,%rdx,8), %rdx
    movq    %rdx, 16(%rdi)
    movq    %rcx, 8(%rdi)
    movq    %rax, (%rdi)
    movq    %rdi, %rax
    movq    %rbp, %rsp
    popq    %rbp
    ret
L98:    movq    %rsp, %rcx
    leaq    -16(%rcx), %rsi
    movq    %rsi, %rsp
    movq    %r8, -16(%rcx)
    jmpq    L150
L117:   movq    %rsp, %rdx
    leaq    -16(%rdx), %rsi
    movq    %rsi, %rsp
    movq    %rcx, -16(%rdx)
    jmpq    L150
L136:   movq    %rsp, %rcx
    leaq    -16(%rcx), %rsi
    movq    %rsi, %rsp
    movq    %rdx, -16(%rcx)
L150:   movabsq $jl_bounds_error_ints, %rcx
    movq    %r9, %rdi
    movl    $1, %edx
    callq   *%rcx

Inbounds

julia> @code_native Base.unsafe_getindex([1,2,3,4],f)
    .text
Filename: /home/steve/.julia/v0.4/GeometryTypes/src/faces.jl
Source line: 16
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 16
    movq    (%rdx), %rax
    movq    8(%rdx), %r8
    movq    (%rsi), %rcx
    movq    -8(%rcx,%rax,8), %rax
    movq    -8(%rcx,%r8,8), %rsi
    movq    16(%rdx), %rdx
    movq    -8(%rcx,%rdx,8), %rcx
    movq    %rcx, 16(%rdi)
    movq    %rsi, 8(%rdi)
    movq    %rax, (%rdi)
    movq    %rdi, %rax
    popq    %rbp
    ret

[dreammaker]

Based on the generated code, what's the main difference between the original version and the new bounds checking version?

[sjkelly]

It doesn't create a GC frame. I haven't run benchmarks but I'll do that soon to confirm it is not allocating. I still don't think this is as good as regular inbound indexing.

[SimonDanisch]

Cool! :) My approach was just a gap filler ... I was actually hoping, that at some point base will support indexing with tuples.

[sjkelly]

Here is my plan: switch to the unsafe version by default since JuliaLang/julia#8227 and JuliaLang/julia#11867 are in PR limbo. Add tests, and a checkbounds(::Mesh) method so developers can avoid errors before they call something in a loop.