Skip to content

cleanups #92

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

cleanups #92

wants to merge 3 commits into from

Conversation

oscardssmith
Copy link
Member

This removes a bunch of compats for unsupported Julia versions, and slightly simplifies the annoying hacks we're making to handle the ABI issues.

@oscardssmith oscardssmith requested a review from DilumAluthge May 13, 2025 21:16
new(data)
end
end
primitive type Float128 <: AbstractFloat 128 end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a breaking change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe bump the version number?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. If we're going to make a breaking release, might as well take the opportunity to do 1.0.0 now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually breaking? I would be very surprised if anyone was using the layout of this type. That said, no objection to making a 1.0 tag...

Copy link
Member

@simonbyrne simonbyrne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a much better idea, I don't know why I didn't do it this way originally.

@oscardssmith
Copy link
Member Author

Also, while I'm cleaning things up, Is the current approach of calling C functions for the basic conversions good? I see ~2x faster performance using the pure Julia UInt128 conversion than the ccalled UInt64 conversion (zen5 linux). If one of you has an aarc64 system (which has native Float128), can you run this benchmark?

julia> @btime Float128(x) setup=x=rand(UInt64)
  10.249 ns (0 allocations: 0 bytes)

@benchmark Float128(UInt128(x)) setup=x=rand(UInt64)
  3.727 ns (0 allocations: 0 bytes)

@simonbyrne
Copy link
Member

On my M2 Mac

julia> @btime Float128(x) setup=x=rand(UInt64)
  2.750 ns (0 allocations: 0 bytes)
5.31167148155784243000000000000000000e+18

julia> @btime Float128(UInt128(x)) setup=x=rand(UInt64)
  2.208 ns (0 allocations: 0 bytes)
6.82926167534499090300000000000000000e+18

Yeah, so that looks faster...

@simonbyrne
Copy link
Member

simonbyrne commented May 14, 2025

You can also clean up these bits as well:

Quadmath.jl/src/Quadmath.jl

Lines 191 to 197 in f6ea87e

if Sys.iswindows()
return reinterpret(Float128,d)
else
y1 = reinterpret(Float64,UInt64(d >> 64))
y2 = reinterpret(Float64,(d % UInt64))
return Float128((VecElement(y2),VecElement(y1)))
end

Quadmath.jl/src/Quadmath.jl

Lines 215 to 221 in f6ea87e

if Sys.iswindows()
return reinterpret(Float128,d)
else
y1 = reinterpret(Float64,UInt64(d >> 64))
y2 = reinterpret(Float64,(d % UInt64))
Float128((VecElement(y2),VecElement(y1)))
end

@oscardssmith
Copy link
Member Author

@simonbyrne I might have gone slightly overboard and moved a lot of the simple implementations into Julia with pretty decent perf gains (usually ~2x sometimes as big as 8x)

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.

3 participants