Fix #411 compile flags on Apple Silicon Ubuntu Docker

[krystophny]

No description provided.

[krystophny]

@andrewgiuliani or @landreman - can you verify if this is fine? A cleaner solution would be a systematic check for compiler options, but the present fix at least makes it compile in our setup.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.71%. Comparing base (e8c5433) to head (f595f84).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
+ Coverage   91.36%   91.71%   +0.34%     
==========================================
  Files          75       75              
  Lines       13150    13150              
==========================================
+ Hits        12015    12061      +46     
+ Misses       1135     1089      -46     

Flag	Coverage Δ
unittests	91.71% <ø> (+0.34%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[landreman]

Looks fine to me. Does anyone else have comments?

[andrewgiuliani]

why are there two branches here, one for aarch64 and another for arm64?

[krystophny]

yeah, this is confusing because "arm64" is given on macos, and "aarch64" on linux on the same CPU. I didn't want to change the old behavior on macos so just added the aarch64 in addition.

[mbkumar]

The clang compiler on Mac (v15.x) now supports march=native. So we need to change the conditions to reflect this. I'll adjust the conditions and push them.

[mbkumar]

I'd recommend to go with the below if else conditions.

if(COMPILER_SUPPORTS_MARCH_NATIVE)
    set(CMAKE_CXX_FLAGS "-O3 -march=native -ffp-contract=fast")
elseif(${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL "arm64")  
    set(CMAKE_CXX_FLAGS "-O3 -mcpu=apple-a14 -ffp-contract=fast")

[mbkumar]

Let me cross check this with what I got for docker build on Mac. Can we also get the docker file uploaded? I'll review this next week after I get back to work. Bharat

…

On Wed, May 15, 2024, 8:27 AM Andrew Giuliani ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In CMakeLists.txt <#412 (comment)> : > @@ -56,9 +56,14 @@ else() unset(COMPILER_SUPPORTS_MARCH_NATIVE CACHE) CHECK_CXX_COMPILER_FLAG(-march=native COMPILER_SUPPORTS_MARCH_NATIVE) if(COMPILER_SUPPORTS_MARCH_NATIVE) - set(CMAKE_CXX_FLAGS "-O3 -march=native -mfma -ffp-contract=fast") - elseif(${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL "arm64") - set(CMAKE_CXX_FLAGS "-O3 -mcpu=apple-a14 -mfma -ffp-contract=fast") + message(STATUS "${CMAKE_HOST_SYSTEM_PROCESSOR}") + if(${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL "aarch64") + set(CMAKE_CXX_FLAGS "-O3 -march=native -ffp-contract=fast") + else() + set(CMAKE_CXX_FLAGS "-O3 -march=native -mfma -ffp-contract=fast") + endif() + elseif(${CMAKE_HOST_SYSTEM_PROCESSOR} STREQUAL "arm64") + set(CMAKE_CXX_FLAGS "-O3 -mcpu=apple-a14 -ffp-contract=fast") why are there two branches here, one for aarch64 and another for arm64? — Reply to this email directly, view it on GitHub <#412 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA62VEGGDKTEHQ3JQCJCYNTZCN5GBAVCNFSM6AAAAABHEHTYYOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANJYGM2TQNRXGY> . You are receiving this because your review was requested.Message ID: ***@***.***>

[krystophny]

Btw the Docker file we are using is ghcr.io/itpplasma/devel-tex based on Debian 12.