Skip to content

Commit a5d41be

Browse files
committed
Eliminate BigInt allocs in Int128 FDs: mul_with_overflow not widemul.
All 128-bit conversions look correct and we've eliminated the allocations. Also, the InexactError error message is nicer now: ``` Before: InexactError(:Int128, Int128, 34028236692093846346337460743176821145500) After: InexactError(:convert, FixedDecimal{Int128, 2}, 0xffffffffffffffffffffffffffffffff) Before: InexactError: Int128(17014118346046923173168730371588410572700) After: InexactError: convert(FixedDecimal{Int128, 2}, 170141183460469231731687303715884105727) ```
1 parent d619b9a commit a5d41be

File tree

2 files changed

+36
-4
lines changed

2 files changed

+36
-4
lines changed

src/FixedPointDecimals.jl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,17 @@ end
296296
Base.convert(::Type{FD{T, f}}, x::FD{T, f}) where {T, f} = x # Converting an FD to itself is a no-op
297297

298298
function Base.convert(::Type{FD{T, f}}, x::Integer) where {T, f}
299-
reinterpret(FD{T, f}, T(widemul(x, coefficient(FD{T, f}))))
299+
C = coefficient(FD{T, f})
300+
throw_inexact() = throw(InexactError(:convert, FD{T, f}, x))
301+
typemin(T) <= x <= typemax(T) || throw_inexact()
302+
xT = convert(T, x) # This won't throw, since we already checked, above.
303+
# Perform x * C, and check for overflow. This is cheaper than a widemul, especially for
304+
# 128-bit T, since those widen into a BigInt.
305+
v, overflow = Base.mul_with_overflow(xT, C)
306+
if overflow
307+
throw_inexact()
308+
end
309+
reinterpret(FD{T, f}, v)
300310
end
301311

302312
Base.convert(::Type{T}, x::AbstractFloat) where {T <: FD} = round(T, x)

test/runtests.jl

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,11 +314,33 @@ end
314314
end
315315
end
316316
@testset "128-bit conversion performance" begin
317-
#@test Base.convert(WFD2, 1) == WFD2(1)
318-
#@test @allocated(Base.convert(WFD2, 1)) == 0
317+
# Baseline cases
318+
@test @allocated(Base.convert(FD2, Int8(-1))) == 0
319+
@test @allocated(Base.convert(FD2, UInt8(1))) == 0
320+
@test @allocated(Base.convert(SFD2, Int8(-1))) == 0
321+
322+
# Int 128 cases
323+
@test @allocated(Base.convert(WFD2, 1)) == 0
324+
@test @allocated(Base.convert(WFD2, 1)) == 0
325+
@test @allocated(Base.convert(UWFD2, 1)) == 0
326+
@test @allocated(Base.convert(WFD2, -1)) == 0
327+
# @test @allocated(Base.convert(UWFD2, -1)) == 32 # not sure how best to test this
328+
@test @allocated(Base.convert(WFD2, UInt(1))) == 0
329+
@test @allocated(Base.convert(UWFD2, UInt(1))) == 0
330+
331+
@test @allocated(Base.convert(WFD2, Int128(1))) == 0
332+
@test @allocated(Base.convert(UWFD2, Int128(1))) == 0
333+
@test @allocated(Base.convert(WFD2, UInt128(1))) == 0
334+
@test @allocated(Base.convert(UWFD2, UInt128(1))) == 0
335+
@test @allocated(Base.convert(WFD2, Int128(-1))) == 0
336+
337+
@test @allocated(Base.convert(FD2, Int128(1))) == 0
338+
@test @allocated(Base.convert(UFD2, Int128(1))) == 0
339+
@test @allocated(Base.convert(FD2, UInt128(1))) == 0
340+
@test @allocated(Base.convert(UFD2, UInt128(1))) == 0
341+
@test @allocated(Base.convert(FD2, Int128(-1))) == 0
319342
end
320343

321-
322344
@testset "promotion" begin
323345
@test 1//10 + FD2(0.1) === 1//5
324346
@test 0.1 + FD2(0.1) === 0.2

0 commit comments

Comments
 (0)