std::isfinite poor codegen with Microsoft STL + Clang
#4609
Labels
Comments
johnstiles-google
changed the title
std::isfinite poor codegen with Clangstd::isfinite poor codegen with Microsoft STL + Clang
On MSVC STL, As shown in #4537, we can fix this when |
|
That's fine. We're happy to #include |
|
I can work on a fix. |
|
That's great! Thank you.
…On Sat, Apr 20, 2024 at 6:39 PM Stephan T. Lavavej ***@***.***> wrote:
I can work on a fix.
—
Reply to this email directly, view it on GitHub
<#4609 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARE6Q2BYHUVKNLR7XSJCRY3Y6LVDXAVCNFSM6AAAAABGQASIU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXHAYDKNJQGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
hubot
pushed a commit
to google/skia
that referenced
this issue
Apr 22, 2024
Chrome and Flutter usage of SkScalarIsFinite has been cleaned up in earlier CLs. `SkIsFinite` continues to exist as private API in SkFloatingPoint.h, in order to avoid regressions in clang-cl. MSVC STL is working on a performance update for their std::isfinite at microsoft/STL#4609. Bug: b/335895731 Bug: b/335901087 Change-Id: Ia9efdf567a2fe9b8653a0dd608ff0804a187d3aa Reviewed-on: https://skia-review.googlesource.com/c/skia/+/844258 Reviewed-by: Brian Osman <[email protected]> Auto-Submit: John Stiles <[email protected]> Commit-Queue: Brian Osman <[email protected]>
|
This fix is expected to ship in VS 2022 17.11 Preview 2. Thanks again for the report! |
|
Thank you for the quick turnaround! Appreciate the support.
…On Fri, Apr 26, 2024 at 7:02 PM Stephan T. Lavavej ***@***.***> wrote:
This fix is expected to ship in VS 2022 17.11 Preview 2. Thanks again for
the report!
—
Reply to this email directly, view it on GitHub
<#4609 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARE6Q2DUZGZYDLZNIEJ2KV3Y7LMHTAVCNFSM6AAAAABGQASIU2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBQGIYTKNZSGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I work on Skia. I recently attempted to replace our hand-rolled implementation of
std::isfinitewith the standard library version, trusting that a modern compiler's implementation would be equally efficient.In practice, this led to a significant, measurable regression in our path-rendering code in clang-cl builds (which mixes Microsoft's STL with the Clang compiler).
I believe the discrepancy comes from https://github.com/microsoft/STL/blob/main/stl/inc/cmath . There does not appear to be support for clang's
__builtin_isfinite. Other Clang builtins are supported, however—there are special codepaths for __builtin_ceilf, __builtin_floorf, __builtin_copysignf, and others. However, isfinite has no Clang builtin support, and ends up calling intofpclassifydfor each value, which is extremely expensive relative to the handful of instructions that a nativeisfiniteneeds.I've filed this with our LLVM team (buganizer) but they said they could not control it from their end.
The text was updated successfully, but these errors were encountered: