Skip to content

Conversation

@Andarwinux
Copy link
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
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
Copy link
Member

kasper93 commented Nov 2, 2025

@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
Copy link

github-actions bot commented Nov 5, 2025

@Andarwinux
Copy link
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
Copy link
Member

kasper93 commented Nov 5, 2025

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
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
Copy link
Member

kasper93 commented Nov 5, 2025

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
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