Skip to content

osdep/compiler: introduce mp_isnan - #16995

Open
Andarwinux wants to merge 1 commit into
mpv-player:masterfrom
Andarwinux:builtin_isnan
Open

osdep/compiler: introduce mp_isnan#16995
Andarwinux wants to merge 1 commit into
mpv-player:masterfrom
Andarwinux:builtin_isnan

Conversation

@Andarwinux

Copy link
Copy Markdown
Contributor

Some platforms like MinGW have suboptimal isnan, so use __builtin_isnan instead if possible.

https://godbolt.org/z/snPKGPfKo

@Andarwinux Andarwinux changed the title osdep/compiler.h: introduce mp_isnan osdep/compiler: introduce mp_isnan Nov 1, 2025
@Andarwinux

Copy link
Copy Markdown
Contributor Author

Using __CRT__NO_INLINE makes things worse, because MinGW's inline isnan in most cases only makes automatic vectorization less efficient, while libcall isnan even makes vectorization impossible.

https://godbolt.org/z/9GbP6dE8a

@kasper93

kasper93 commented Nov 2, 2025

Copy link
Copy Markdown
Member

@mstorsjo: Do you think we could improve situation upstream? Currently we are using inline impl from mingw. With __CRT__NO_INLINE=1 we can jmp to CRT isnan, but generally speaking would be nice to use compiler generated impl if possible, no?

Some platforms like MinGW have suboptimal isnan, so use __builtin_isnan instead if possible.
@github-actions

github-actions Bot commented Nov 5, 2025

Copy link
Copy Markdown

@Andarwinux

Copy link
Copy Markdown
Contributor Author

Can we continue? Even if MinGW makes such changes internally, it would take a long time for them to reach distributions, but custom isnan would be available immediately.

@kasper93

kasper93 commented Nov 5, 2025

Copy link
Copy Markdown
Member

I don't know. What are we fixing here really? Is there critical performance issue that needs to be fixed or is this microoptimization? I agree it can be improved, but unless there is good reason I don't see why we need to do it in mpv, and not in toolchain.

What about libplacebo or ffmpeg?

@Andarwinux

Copy link
Copy Markdown
Contributor Author

What about libplacebo or ffmpeg?

libplacebo does not use isnan, while ffmpeg is already using a custom isnan.

Speaking of performance, I initially discovered this issue in mujs and svtav1-psy, where a 5% improvement was observed in benchmarks (on top of PGO+LTO).

@kasper93

kasper93 commented Nov 5, 2025

Copy link
Copy Markdown
Member

Speaking of performance, I initially discovered this issue in mujs and svtav1-psy, where a 5% improvement was observed in benchmarks (on top of PGO+LTO).

I don't think in mpv you would see any measurable difference. I would rather focus on bottlenecks that we have. One of which is currently libass, if you want to profile and optimize that.

@Andarwinux

Copy link
Copy Markdown
Contributor Author

Speaking of performance, I initially discovered this issue in mujs and svtav1-psy, where a 5% improvement was observed in benchmarks (on top of PGO+LTO).

I don't think in mpv you would see any measurable difference. I would rather focus on bottlenecks that we have. One of which is currently libass, if you want to profile and optimize that.

Yes, I also plan to do this for libass later (it does have a noticeable impact). But there's no harm in doing it in mpv.

@sfan5 sfan5 added the priority:on-ice may be revisited later label Dec 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:on-ice may be revisited later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants