Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Apr 18, 2025

Currently, we check for iszero(_add.alpha) in various matrix multiplication functions, where _add::MulAddMul. However, a MulAddMul(alpha, beta) stores isone(alpha) as a type parameter, and if alpha is one, we know at compile-time that iszero(_add.alpha) is false. Using the type parameter in dispatch would allow us to eliminate the iszero(alpha) branches.

@codecov
Copy link

codecov bot commented Apr 18, 2025

Codecov Report

Attention: Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.03%. Comparing base (61e444d) to head (6fc77c1).

Files with missing lines Patch % Lines
src/generic.jl 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1293      +/-   ##
==========================================
- Coverage   92.03%   92.03%   -0.01%     
==========================================
  Files          34       34              
  Lines       15501    15503       +2     
==========================================
+ Hits        14267    14268       +1     
- Misses       1234     1235       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dkarrasch
Copy link
Member

Does this improve TTFX visibly?

@jishnub
Copy link
Member Author

jishnub commented Apr 19, 2025

It helps a bit when the branch calls _rmul_or_fill!, which may be skipped.

julia> n = 4; T = t = Bidiagonal(ones(n), ones(n-1), :U); C = similar(T); D = Diagonal(T);

julia> @time mul!(C, T, D);
  0.124931 seconds (398.97 k allocations: 20.078 MiB, 99.98% compilation time) # master
  0.110213 seconds (387.34 k allocations: 19.559 MiB, 99.98% compilation time) # this PR

Often, the different would be small, as the branch is trivial.

@jishnub jishnub added the ttfx The change pertains to first-call latency label Apr 19, 2025
@jishnub jishnub merged commit cab0dc6 into master Apr 22, 2025
1 of 2 checks passed
@jishnub jishnub deleted the jishnub/iszeroalpha branch April 22, 2025 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ttfx The change pertains to first-call latency

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants