JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.78k stars 5.49k forks source link

Addition of `Float16` is wrong on armv7l #41064

Open giordano opened 3 years ago

giordano commented 3 years ago
julia> Float16(-3.743e-5) + Float16(1)
Float16(3.05e-5)

@code_native Float16(-3.743e-5) + Float16(1):

        .text
; ┌ @ float.jl:324 within `+'
        push    {r11, lr}
        vpush   {d8}
        vmov    r0, s1
        vmov.f32        s16, s0
        bl      ($a)
        vmov    r1, s16
        vmov    s16, r0
        mov     r0, r1
        bl      ($a)
        vmov    s0, r0
        vadd.f32        s0, s0, s16
        vmov    r0, s0
        bl      ($a)
        uxth    r0, r0
        vmov    s0, r0
        vpop    {d8}
        pop     {r11, pc}

@code_llvm Float16(-3.743e-5) + Float16(1):

;  @ float.jl:324 within `+'
define half @"julia_+_848"(half %0, half %1) {
top:
  %2 = fpext half %0 to float
  %3 = fpext half %1 to float
  %4 = fadd float %2, %3
  %5 = fptrunc float %4 to half
  ret half %5
}

Spotted in https://github.com/JuliaMath/SpecialFunctions.jl/pull/326

giordano commented 3 years ago

Suggested by @staticfloat:

julia> foo(x::Float16, y::Float16) = x + y
foo (generic function with 1 method)

julia> foo(Float16(-3.743e-5), Float16(1))
Float16(1.0)

@code_native foo(Float16(-3.743e-5), Float16(1)):

        .text
; ┌ @ REPL[16]:1 within `foo'
        vcvtb.f32.f16   s2, s1
        vcvtb.f32.f16   s0, s0
; │┌ @ float.jl:324 within `+'
        vadd.f32        s0, s0, s2
        vcvtb.f16.f32   s0, s0
        vmov    r0, s0
        uxth    r0, r0
; │└
        vmov    s0, r0
        bx      lr
; └

@code_llvm foo(Float16(-3.743e-5), Float16(1)):

;  @ REPL[16]:1 within `foo'
define half @julia_foo_862(half %0, half %1) {
top:
; ┌ @ float.jl:324 within `+'
   %2 = fpext half %0 to float
   %3 = fpext half %1 to float
   %4 = fadd float %2, %3
   %5 = fptrunc float %4 to half
; └
  ret half %5
}
staticfloat commented 3 years ago

Wrapping it in a function and toggling compile=no versus compile=yes, we can get different results:

$ julia-1.6.1/bin/julia --compile=no -E 'function foo(); return Float16(-3.743e-5) + Float16(1); end; foo()' 2>/dev/null
Float16(3.05e-5)
$ julia-1.6.1/bin/julia --compile=yes -E 'function foo(); return Float16(-3.743e-5) + Float16(1); end; foo()' 2>/dev/null                                                                                                                                                 
Float16(1.0)

@Keno or @JeffBezanson do you think this could be an issue with the interpreter or constant-propagation somehow?

oscardssmith commented 1 year ago

Does this still reproduce? A lot of our Float16 handling has been rewritten since 2021.

giordano commented 1 year ago

I never got to use Julia on hardware armv7l/armv6l lately, especially because building Julia on these architectures has been broken since v1.8 or so.

oscardssmith commented 1 year ago

in that case... bug fixed?

giordano commented 1 year ago

"making Julia not build at all" is an interesting definition of "fixing a bug".

oscardssmith commented 1 year ago

Isn't thinking outside the box fun?

giordano commented 1 year ago

Not really.