Clipper2: Unexpected collinearity test failure on M1 macOS

Just my 5 cents, maybe this can help you somehow, because I have some experience in working with M1 macs, here are some potential differencies that can cause troubles:

• all FP math works the same way as on AMD64, but complex functions like trigonometry and logarithms are producing slightly different results even between AMD64 and M1 macs, of course it is also different in Mac vs Linux vs Windows because of different implementations of C library:

Mac M1: std::cos(-1.5446728605250191) = 0.026120495107843492
Mac x64: std::cos(-1.5446728605250191) = 0.026120495107843495

(Sidenote: std::sqrt is producing perfectly equal results on all sane platforms)

• by default FP math is optimized aggressively for M1 macs, Adding -ffp-contract=off to compile arguments makes it similar to AMD64 macs.
• For any code compiled with Clang (or maybe only AppleClang, I am not sure) argument evaluation order in function calls is opposite to GCC/MSVC. C++ standard does not define argument evaluation order, but still GCC/MSVC tries to evaluate arguments from right to left, and AppleClang does it from left to right.
• std::sort can crash if compare predicate is not strict-weak ordering, that is an old “problem” of libc++'s implementation of std::sort.
• RTTI implementation on all macs is completely abhorrent, it is unable to merge 2 implementations of the same polymorphic type into one, if two implementations are coming from different dynamic libraries. But this is not the case for Clipper of course.

In your place I would first try to find trigonometric function that introduces difference, but I am not sure if clipper uses any for simple clipping (I believe it uses sin and cos for offsetting, but we are not talking about offsetting). Second option is to try adding -ffp-contract=off and checking if issue goes away. If it does - some multiply-add operation is producing slightly different result and this reveals a bug.

ADDED: Forgot to mention, most of the time all issues except first two are reproducible on any CPU and OS with Clang + libc++ build. You can try reproducing it like this as well, maybe it will work.

As for C++, I have updated the PR to use unsigned integers, for which the result should be well-defined: the values should just wrap (regardless of compiler settings it seems), which I believe is exactly what we want. I have also added a simple test case (which would fail for the current version that uses double).

However, I’m not sure how the various compilers will respond when there are overflow errors.

Yeah, that’s a good point. I think at minimum, there should be a test case that gets executed as part of the GitHub Actions tests. I could try and add one for C++?

@AngusJohnson Did you try to reproduce it like this: [#777 (comment)]

No I didn’t. Sorry. And looking at this problem again I can now see a very simple solution.

  inline bool IsCollinear(const Point64& pt1, 
    const Point64& sharedPt, const Point64& pt2) // #777
  {
    const auto a = static_cast<double>(sharedPt.x - pt1.x);
    const auto b = static_cast<double>(pt2.y - sharedPt.y);
    const auto c = static_cast<double>(sharedPt.y - pt1.y);
    const auto d = static_cast<double>(pt2.x - sharedPt.x);
    return a * b == c * d;
  }

I was wondering why this was not defaulting to =off and found this: https://releases.llvm.org/14.0.0/tools/clang/docs/ReleaseNotes.html#floating-point-support-in-clang

So it is fnmsub instruction in M1 build that is causing a difference, and it is the CrossProduct function that ends up having this opcode. I added some debug info to it and started the test:

  template <typename T>
  inline double CrossProduct(const Point<T>& pt1, const Point<T>& pt2, const Point<T>& pt3) {
    const double d = (static_cast<double>(pt2.x - pt1.x) * static_cast<double>(pt3.y -
      pt2.y) - static_cast<double>(pt2.y - pt1.y) * static_cast<double>(pt3.x - pt2.x));
    
    std::cout.precision(20);
    std::cout << "Inputs:" << std::endl;
    std::cout << pt1.x << ", " << pt1.y << std::endl;
    std::cout << pt2.x << ", " << pt2.y << std::endl;
    std::cout << pt3.x << ", " << pt3.y << std::endl;
    std::cout << "Cross product:" << std::endl;
    std::cout << d << std::endl;
    return d;
  }

And here is the difference between M1 build (on the left side) and AMD64 build (on the right) https://www.diffchecker.com/kaHSyAnl/

I guess to reproduce this problem on a normal AMD64 Angus can try to use those results from the left side of the diff.