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

@fastmath not applying to additions sometimes #108

Closed
Zentrik opened this issue Mar 26, 2023 · 3 comments
Closed

@fastmath not applying to additions sometimes #108

Zentrik opened this issue Mar 26, 2023 · 3 comments

Comments

@Zentrik
Copy link
Contributor

Zentrik commented Mar 26, 2023

function test5(x, y, z) 
    @fastmath @inbounds for i in eachindex(x)
        return x[i] + y[i] + z[i]
    end
end

a = [Vec{8, Float32}(0)]
@code_llvm test5(a, a, a)

We see that

; ┌ @ fastmath.jl:265 within `add_fast`
; │┌ @ operators.jl:591 within `+` @ C:\Users\rag\.julia\packages\SIMD\7eukp\src\simdvec.jl:253
; ││┌ @ C:\Users\rag\.julia\packages\SIMD\7eukp\src\LLVM_intrinsics.jl:212 within `fadd` @ C:\Users\rag\.julia\packages\SIMD\7eukp\src\LLVM_intrinsics.jl:212
; │││┌ @ C:\Users\rag\.julia\packages\SIMD\7eukp\src\LLVM_intrinsics.jl:221 within `macro expansion`
      %16 = fadd <8 x float> %.unpack, %.unpack10
      %17 = fadd <8 x float> %16, %.unpack11

For whatever reason doing 2 additions on the same line in a for loop removes the fast flag from fadd.

@Zentrik
Copy link
Contributor Author

Zentrik commented Nov 7, 2023

Ok identified what the problem is

julia> @macroexpand test(x, y, z) = @fastmath x[0] + y[0] + z[0]
 :(test(x, y, z) = begin
	#= REPL[45]:1 =#
	Base.FastMath.add_fast(x[0], y[0], z[0])
end) 

The problem is we're calling https://github.com/JuliaLang/julia/blob/e7345b89fd4eb15e8f395395701e19be705d7b06/base/fastmath.jl#L263-L264.

		# fall-back implementation for non-numeric types
        $op_fast(xs...) = $op(xs...)

We probably want something like
https://github.com/JuliaLang/julia/blob/e7345b89fd4eb15e8f395395701e19be705d7b06/base/fastmath.jl#L168-L169.

add_fast(x::T, y::T, zs::T...) where {T<:FloatTypes} = 
     add_fast(add_fast(x, y), zs...) 

@KristofferC
Copy link
Collaborator

n-arg parsing strikes again...

KristofferC pushed a commit to JuliaLang/julia that referenced this issue May 29, 2024
Currently using the fastmath vararg +, *, min, max methods only actually
sets fastmath if they are specifically overloaded even when the correct
2 argument methods have been defined.
As such, `ComplexF32, ComplexF64` do not currently set fastmath when
using the vararg methods. This will also fix any other types, such as
those in SIMD.jl, which don't overload the vararg methods.

E.g. 
```julia
x = ComplexF64(1)
f(x) = @fastmath x + x + x
```
now works correctly.

I see no reason why the vararg methods shouldn't default to using the
fastmath 2 argument methods instead of the non fastmath ones, which is
the current behaviour.

I also switched the implementation to use `afoldl` as that's what the
non fastmath vararg methods use.

Fixes #54456 and eschnett/SIMD.jl#108.
@KristofferC
Copy link
Collaborator

Fixed by JuliaLang/julia#54513 (on 1.12) at least.

DilumAluthge pushed a commit to JuliaLang/julia that referenced this issue Jun 3, 2024
Currently using the fastmath vararg +, *, min, max methods only actually
sets fastmath if they are specifically overloaded even when the correct
2 argument methods have been defined.
As such, `ComplexF32, ComplexF64` do not currently set fastmath when
using the vararg methods. This will also fix any other types, such as
those in SIMD.jl, which don't overload the vararg methods.

E.g. 
```julia
x = ComplexF64(1)
f(x) = @fastmath x + x + x
```
now works correctly.

I see no reason why the vararg methods shouldn't default to using the
fastmath 2 argument methods instead of the non fastmath ones, which is
the current behaviour.

I also switched the implementation to use `afoldl` as that's what the
non fastmath vararg methods use.

Fixes #54456 and eschnett/SIMD.jl#108.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants