math icon indicating copy to clipboard operation
math copied to clipboard

Published 20 hours ago • verified publisher icon boostorg

FIX: avoid overflow on overflow check for Mac M1

Open mckib2 opened this issue 2 years ago • 9 comments

  • x-ref https://github.com/scipy/scipy/pull/17432
  • avoid calculating [number]*tools::max_value<T>() to prevent overflows on Apple M1 processors

mckib2 avatar Feb 05 '23 05:02 mckib2

@mborland @jzmaddock Didn't realize the changes would be similar for all issues at the start, so consolidating all these small PRs into a single one

mckib2 avatar Feb 05 '23 05:02 mckib2

I believe SciPy is using this image and seeing the errors. It's about 9 months old and I wonder if updating to the latest may avoid these (compiler?) issues/bugs

mckib2 avatar Feb 05 '23 22:02 mckib2

I believe SciPy is using this image and seeing the errors. It's about 9 months old and I wonder if updating to the latest may avoid these (compiler?) issues/bugs

Might be worth updating. I am using macOS Ventura with Clang 14.0.0 on an M1 MacBook Pro and see none of the errors. Anyone on Ventura won't even have the option for the Clang 13 series.

mborland avatar Feb 05 '23 22:02 mborland

I confess I'm liking this less and less: the code appears to be correct, something somewhere is generating incorrect code if spurious overflow is happening on a code branch that should never be taken. Plus volatile plays absolute havoc with compiler optimizers. I don't as it happens mind at all if this is the full extent of the issue, but I have a hunch that we're about to take a deep dive through a very deep rabbit hole, as the library is choc full of logic like this.

So I guess the questions are:

  • is this fixed in the latest release as Matt suggests?
  • If not, how come Matt and I are unable to reproduce? There still appears to be something critical missing.
  • What's the Apple/Clang take on this? Is it a known issue upstream?

jzmaddock avatar Feb 06 '23 17:02 jzmaddock

is this fixed in the latest release as Matt suggests?

Testing this out now

If not, how come Matt and I are unable to reproduce? There still appears to be something critical missing.

Sounds like neither of you have tried on this specific version of MacOS? I also notice it only shows up when -O2 or -O3 optimizations are enabled.

What's the Apple/Clang take on this? Is it a known issue upstream?

I wasn't able to find anything upstream, I'll submit an issue

mckib2 avatar Feb 12 '23 22:02 mckib2

What's the Apple/Clang take on this? Is it a known issue upstream?

I wasn't able to find anything upstream, I'll submit an issue

If you are able to make an extremely minimal reproducer like the GCC Bugzilla requires @NAThompson can route it to the right people at Apple.

mborland avatar Feb 12 '23 22:02 mborland

What's the Apple/Clang take on this? Is it a known issue upstream?

I wasn't able to find anything upstream, I'll submit an issue

If you are able to make an extremely minimal reproducer like the GCC Bugzilla requires @NAThompson can route it to the right people at Apple.

Minimal reproducer (not sure how it compares to GCC Bugzilla) is in the upstream llvm-project issue: https://github.com/llvm/llvm-project/issues/60695#issue-1581432780

mckib2 avatar Feb 13 '23 00:02 mckib2

If you are able to make an extremely minimal reproducer like the GCC Bugzilla requires @NAThompson can route it to the right people at Apple.

@mborland The issue was rejected by Clang maintainers, I'll try looking up my Apple ID and submitting an issue there

mckib2 avatar Feb 15 '23 07:02 mckib2

Since the issue seems to be fixed in newer macOS I wouldn't be surprised if they don't investigate the bug either.

mborland avatar Feb 15 '23 20:02 mborland