Skip to content

BUG: Fix overflow warning on apple silicon - #18571

Merged
mattip merged 2 commits into
numpy:mainfrom
isuruf:overflow
Mar 17, 2021
Merged

mattip merged 2 commits into
numpy:mainfrom
isuruf:overflow

Conversation

@isuruf

@isuruf isuruf commented Mar 7, 2021

Copy link
Copy Markdown
Contributor

Alternative to #17712

@seberg

seberg commented Mar 7, 2021

Copy link
Copy Markdown
Member

@isuruf, I know that did not work in gh-18555 which may actually just be the identical issue?

Before we add more clang workaround, can you make sure to compiling with -ffp-exception-behavior=strict? We have to make that flag default on clang and make sure to not add any workarounds for clang bugs that are actually just bad compiler defaults.

I am not on top of these efforts here, so ignore if this is obviously unhelpful...

@isuruf

isuruf commented Mar 7, 2021

Copy link
Copy Markdown
Contributor Author

-ffp-exception-behavior=strict

The exception is still there if I add that flag without this PR.

@rgommers

Copy link
Copy Markdown
Member

The one PyPy failure seems unrelated:

____________________ TestRandomDist.test_choice_exceptions _____________________
...
TypeError: __format__() missing 1 required positional argument: 'spec'

Let's see if there's more opinions on a preference of this PR vs. gh-17712. I'd be inclined to merge this one.

@erykoff

erykoff commented Mar 13, 2021

Copy link
Copy Markdown

I think this would be fine. I think my solution on #17712 would be a bit cleaner (and have the advantage of unifying the code path of single-value and vector computations), but I was stymied by the time taken to make it work with the 128-bit code path. This is certainly a simpler and more direct solution.

Comment thread numpy/core/src/umath/loops.c.src Outdated
Co-authored-by: Matti Picus <[email protected]>
@mattip
mattip merged commit ee9c8f8 into numpy:main Mar 17, 2021
@isuruf
isuruf deleted the overflow branch March 25, 2021 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants