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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants