completing a phase #43
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43 +/- ##
==========================================
- Coverage 66.66% 64.94% -1.72%
==========================================
Files 3 3
Lines 186 194 +8
==========================================
+ Hits 124 126 +2
- Misses 62 68 +6 ☔ View full report in Codecov by Sentry. |
|
Do we want to get this merged? |
|
I resolved the conflicts. |
|
here are the resolutions |
|
This PR still shows 3 files with conflicts. No merge button. |
This reverts commit d6f43f7.
|
@oscardssmith @maleadt Since you guys have been on a tear - what about this one? |
| Base.atan(y::BFloat16, x::BFloat16) = BFloat16(atan(Float32(y), Float32(x))) | ||
| Base.hypot(x::BFloat16, y::BFloat16) = BFloat16(hypot(Float32(x), Float32(y))) | ||
| Base.hypot(x::BFloat16, y::BFloat16, z::BFloat16) = BFloat16(hypot(Float32(x), Float32(y), Float32(z))) | ||
| Base.clamp(x::BFloat16, lo::BFloat16, hi::BFloat16) = BFloat16(clamp(Float32(x), Float32(lo), Float32(hi))) |
There was a problem hiding this comment.
Definitions like this are not beneficial. The generic definitions work fine already: https://github.com/JuliaLang/julia/blob/48ddd2dc2859f41fc1a82876cc5654fb31d7b2ba/base/intfuncs.jl#L1269-L1272
julia> clamp(BFloat16(5), BFloat16(5.5), BFloat16(6.5))
BFloat16(5.5)By eagerly demoting to FLoat32 you're making it impossible for LLVM to emit instructions that would support operations like this, which is now possible after JuliaLang/julia#55486.
Similar arguments can be made for other operations.
There was a problem hiding this comment.
Ok, so this pre-dates the LLVM support. Should we clean up this package and perhaps remove the other operations of this nature?
There was a problem hiding this comment.
Closing this PR in that case.
No description provided.