Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer vector comparison is working incorrectly on Intel based Macs with MacOS >= 11 #61

Open
egorodet opened this issue Jul 18, 2022 · 5 comments
Assignees
Labels

Comments

@egorodet
Copy link
Contributor

egorodet commented Jul 18, 2022

Signed and unsigned integer vectors comparison operators (==, <, >) seem to be working incorrectly in some cases on Intel-based Macs. This is reproducible in CI system of my project which is testing wrapper class Point<T,size> around HLSL++ vector in Azure Pipelines running on Macs with x86_64 architecture. Please see build 20220716.1 (on MacOS 11) and build 20220716.2 (on MacOS 12) test results. I noticed these test failures after switching build agents to MacOS v11, while previously on MacOS 10.15 tests were passing successfully - see build 20220715.1. I don't have Intel based Mac under my hands right now, so I can't check it locally, but I believe that comparison operator tests added in PR #59 will help to reproduce this issue.

egorodet added a commit to MethanePowered/MethaneKit that referenced this issue Jul 19, 2022
Version 0.6.4: Hot-Fixes

- **Samples** apps:
  - Attempted replacement of `PerlinNoise` with `FastNoise2` library, but reverted it back because of no ARM support for M1 Macs (see [FastNoise2 issue](Auburn/FastNoise2#93))
- **Graphics** libraries:
  - Fixed runtime errors in GPU profiling builds with `METHANE_GPU_PROFILING_ENABLED=ON`:
    - DirectX GPU timestamps re-calibration issue leading to GPU ranges shifting was fixed.
    - Fixed command list execution waiting threads synchronisation in Profiling builds in Typography tutorial.
  - Fixed resources retaining in command lists. Retained resources were incorrectly cleared on `CommandList::Reset()`, while they should be cleared on `CommandList::Commit()`.
  - Fixed DirectX descriptor heaps allocations after `Context::Reset()` by always using deferred heap allocation in all cases. Deferred heap initialisation flag was removed, since it became unconditionally deferred.
  - Fixed sporadic hang in `CommandQueueTrackingBase::WaitForExecution()`
  - Fixed sporadic crash on destruction of `CommandQueueTrackingBase` with proper shutdown procedure called from destructor of derived class.
  - Fixed Vulkan build and some initialisation errors on MacOS.
- **Tests**:
  - All unit tests were updated to support breaking changes in Catch v3.
  - `Point<T,size>` wrapper class was extended with workarounds of MacOS & ARM specific bugs in HLSL++ integer vector comparison and division operators (see [1](redorav/hlslpp#60) and [2](redorav/hlslpp#61)).
  - Logging of `Point`, `Rect` and `RectSize` values was added in `DataTypes` unit tests.
- **External** libraries:
  - External dependencies management via Git submodules was replaced with [CPM.cmake](https://github.com/cpm-cmake/CPM.cmake) package manager. No submodules anymore - it greatly simplifies external library updates!
    - All externally dependent repositories are downloaded to CPM cache directory during CMake configuration stage, to `Build/Output/ExternalsCache` by default (it can be changed with CMake option `CPM_SOURCE_CACHE`).
    - When CMake project is configured under CLion, external repositories are downloaded to individual build directories of each configuration to workaround [parallel cache update collision issue of the CPM.cmake](cpm-cmake/CPM.cmake#293).
  - New `README.md` description of the external dependencies was added in `Externals` directory.
  - Almost all libraries were updated to latest version, except SPIRV-Cross and DirectX Shader Compiler pre-built binary tools.
- **Build** infrastructure:
  - Root `CMakeLists.txt` was simplified by moving all compiler configuration options to `CMake/MethaneBuildOptions.cmake`.
  - Use caching of Externals CPM package sources in Azure Pipelines and GitHub Workflows to speedup builds.
  - MacOS builders were switched from v10.15 to v11.
@redorav
Copy link
Owner

redorav commented Jul 22, 2022

Unfortunately I cannot seem to repro this on my Android phone (ARM 64 configuration) so I'm a bit at a loss as to what can be going wrong here. Also the equality operator is pretty simple. Can you give a bit more detail on this?

@egorodet
Copy link
Contributor Author

This issue is reproducible on Intel x86_64 architecture (not ARM), and only on MacOS >= 11. As I mentioned, it was working fine on MacOS 10.15. It looks very strange because the same SSE-based integer vectors comparison works fine on Windows and Linux. I don't know why it happens, but it is proved with unit testing results in Azure Devops available by the links in previous message.

@redorav
Copy link
Owner

redorav commented Jul 22, 2022

Ah I see what you mean. I'll have to get hold of a machine like you describe somehow then, or maybe a virtual machine.

@redorav redorav self-assigned this Jul 22, 2022
@redorav redorav added the bug label Jul 22, 2022
@redorav
Copy link
Owner

redorav commented Sep 25, 2024

@egorodet Does it still make sense to keep this open?

@egorodet
Copy link
Contributor Author

@egorodet Does it still make sense to keep this open?

Let me check this. Intel-based macOS machines are still available in GitHub hosted runners for GH Actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants