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

use muladd in horner macro #9881

Merged
merged 3 commits into from
Feb 4, 2015
Merged

Conversation

simonbyrne
Copy link
Contributor

Polynomials should now be able to take advantage of fma operations when available (cf. #9840).

cc: @stevengj, @eschnett

@ivarne
Copy link
Member

ivarne commented Jan 22, 2015

Test failiure:

ERROR: LoadError: LoadError: test error in expression: @evalpoly 2 3 4 5 6 == 3 + 2 * (4 + 2 * (5 + 2 * 6)) == @evalpoly 2 + 0im 3 4 5 6
muladd not defined for Int64
while loading math.jl, in expression starting on line 327
while loading /tmp/julia/share/julia/test/runtests.jl, in expression starting on line 42

Should we have a default fallback for muladd(::Number, ::Number, ::Number)?

@simonbyrne
Copy link
Contributor Author

Yes, there should be a fallback.

@eschnett
Copy link
Contributor

The line

muladd{T<:Number}(x::T, y::T, z::T) = no_op_err("muladd", T)

in promotion.cl should instead read

muladd{T<:Number}(x::T, y::T, z::T) = x*y+z

@eschnett
Copy link
Contributor

I have opened #9882 that also adds test cases for muladd.

@stevengj
Copy link
Member

Shouldn't we also use it in @evalpoly, which also has multiply-accumulate operations on real operands? (Except that it has $ai * tt + $b, $b + r*$ai, and $(esc(p[i])) - s * $ai, where the last is an fnma operation. Just muladd is not enough, as I've pointed out several times since #6330.)

@stevengj stevengj mentioned this pull request Jan 22, 2015
@simonbyrne
Copy link
Contributor Author

Ah, good point: I forgot about @evalpoly.

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

Successfully merging this pull request may close these issues.

6 participants