Skip to content

PERF: replace use of isnan once MSVC bug is fixed #23209

Closed
@chris-b1

Description

@chris-b1
Contributor

ref #23182, #21813

MSVC bug tracker - https://developercommunity.visualstudio.com/content/problem/294290/incorrect-codegen-for-double-equality.html

Once that bug is fixed in a widely available MSVC for python 3 builds we should redefine isnan / notnan to be based on an equality check here.
https://github.com/pandas-dev/pandas/pull/23182/files#diff-467e9a847ab859a085518bc17711fe57R20

Current code works around that MSVC bug by using the libc, isnan, which works, but can't be inlined, so likely has a little perf cost
https://godbolt.org/z/y550lB

Activity

jbrockmendel

jbrockmendel commented on Oct 30, 2019

@jbrockmendel
Member

@chris-b1 any idea if this has been addressed upstream?

chris-b1

chris-b1 commented on Nov 7, 2019

@chris-b1
ContributorAuthor

That bug has been fixed in the most recent version of MSVC - I'm a little disconnected form our build process to know if that's what were using.

jbrockmendel

jbrockmendel commented on Nov 8, 2019

@jbrockmendel
Member

most recent version of MSVC

Do you know what that version is? In _libs/src/cmath there is a comment "Place upper bound on this check once a fixed MSVC is released"

added a commit that references this issue on Nov 8, 2019
269b2e6
TomAugspurger

TomAugspurger commented on Nov 8, 2019

@TomAugspurger
Contributor
jbrockmendel

jbrockmendel commented on Dec 21, 2021

@jbrockmendel
Member

do we have a way of checking if this is actionable?

added this to the 1.4 milestone on Dec 22, 2021
modified the milestones: 1.4, 1.5 on Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    PerformanceMemory or execution speed performanceWindowsWindows OS

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      Participants

      @jreback@TomAugspurger@chris-b1@jbrockmendel@lithomas1

      Issue actions

        PERF: replace use of `isnan` once MSVC bug is fixed · Issue #23209 · pandas-dev/pandas