Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

im*x is inefficient #12851

Closed
eschnett opened this issue Aug 28, 2015 · 5 comments
Closed

im*x is inefficient #12851

eschnett opened this issue Aug 28, 2015 · 5 comments
Labels
complex Complex numbers performance Must go faster

Comments

@eschnett
Copy link
Contributor

The mathematical expression i x is naturally translated to im*x. Unfortunately, this leads to less efficient code than 1im*x. (This is with LLVM 3.6.1).

julia> f(x)=im*x; @code_native f(1.0)
    .section    __TEXT,__text,regular,pure_instructions
Filename: none
Source line: 0
    pushq   %rbp
    movq    %rsp, %rbp
    vmovaps %xmm0, %xmm1
Source line: 1
    vmovq   %xmm1, %rax
    vxorps  %xmm0, %xmm0, %xmm0
    testq   %rax, %rax
    jns L36
    movabsq $13406539760, %rax      ## imm = 0x31F178FF0
    vmovsd  (%rax), %xmm0
L36:    popq    %rbp
    retq
julia> f(x)=1im*x; @code_native f(1.0)
    .section    __TEXT,__text,regular,pure_instructions
Filename: none
Source line: 0
    pushq   %rbp
    movq    %rsp, %rbp
    vmovaps %xmm0, %xmm1
    vxorps  %xmm0, %xmm0, %xmm0
Source line: 1
    vmulsd  %xmm0, %xmm1, %xmm0
    popq    %rbp
    retq

That is, while the expression 1im*x leads to ideal code (I guess the multiplication by zero has to remain because of nans?), the expression im*x contains a branch, moving values between xmm and general registers, and loads the constant 0.0 from memory (!). Also, the nan semantics are different, and I'd argue they are wrong -- I expect Nan+Nan*im as result:

julia> (im*NaN, 1im*NaN)
(0.0 + NaN*im,NaN + NaN*im)

My guess is that the solution is either to ensure that Complex{Bool} is converted to Complex{Int} before being converted to Complex{Float64}, or to explicitly define complex arithmetic operators that handle Complex{Bool}.

@jiahao
Copy link
Member

jiahao commented Aug 28, 2015

Relevant: #10000 - the redesign of im as Complex{Bool} unfortunately also no longer respects sign of zero, which can be important when dealing with branch cuts.

@eschnett
Copy link
Contributor Author

I just see that the special case is already there:

*(x::Bool, z::Complex) = ifelse(x, z, zero(z))

I don't think it should use ifelse, at least not for Float32 and Float64.

@yuyichao
Copy link
Contributor

Just one more datapoint. im * x is not inlined for some cases (Float32 or Complex64) which disables @simd while 1im is fine in such cases.

@kshyatt kshyatt added performance Must go faster complex Complex numbers labels Aug 28, 2015
@eschnett
Copy link
Contributor Author

These additional definitions

*(x::Real, z::Complex{Bool}) = Complex(x*real(z), x*imag(z))
*(z::Complex{Bool}, x::Real) = Complex(real(z)*x, imag(z)*x)

seem to solve this issue for me. These definitions catch im, which is of the type Complex{Bool}; they mirror the existing special cases for Bool * Complex.

@jakebolewski
Copy link
Member

Closed by #12887

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex Complex numbers performance Must go faster
Projects
None yet
Development

No branches or pull requests

5 participants