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

Windows build with clang-cl; CI for Windows/ARM64 build #1538

Merged
merged 5 commits into from
May 13, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Apr 18, 2024

Issues

Needed for: aws/aws-lc-rs#376

Description

  • Updated CMakeLists.txt to support a Windows build using clang-cl.
  • Adds CI build/test of Windows/x64 using clang-cl.
  • Adds CI build of Windows/ARM64 using clang-cl. (Currently there are no runners available for testing Windows/ARM64 executables.)

Callout

  • This change removes debug information by default from the "Release" build type. Alternatively, one may retain debug information in the artifacts by adding the -ggdb build flag via CMAKE_C_FLAGS or by using the "Relwithdebinfo" or "Debug" build types.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth force-pushed the windows-clang-cl branch 27 times, most recently from 8c2e497 to 2fec18c Compare April 22, 2024 13:00
@justsmth justsmth changed the title [DRAFT] Windows build with clang-cl Windows build with clang-cl; CI for Windows/ARM64 build Apr 22, 2024
@justsmth justsmth marked this pull request as ready for review April 22, 2024 13:10
smittals2
smittals2 previously approved these changes Apr 23, 2024
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
smittals2
smittals2 previously approved these changes Apr 30, 2024
samuel40791765
samuel40791765 previously approved these changes Apr 30, 2024
diff --git a/.github/workflows/windows-alt.yml b/.github/workflows/windows-alt.yml
index 5d808ad19..e25ef8350 100644
--- a/.github/workflows/windows-alt.yml
+++ b/.github/workflows/windows-alt.yml
@@ -63,12 +63,64 @@ jobs:
           options: |
             CMAKE_SYSTEM_NAME=Windows \
             CMAKE_SYSTEM_PROCESSOR=x86_64 \
-            CMAKE_BUILD_TOOL=ninja.exe \
+            CMAKE_MAKE_PROGRAM=ninja.exe \
             CMAKE_BUILD_TYPE=Release \
       - name: Build Project
         run: cmake --build ./build --target all
       - name: Run tests
         run: cmake --build ./build --target run_tests
+  clang-cl:
+    if: github.repository_owner == 'aws'
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+          - x64
+          - x64_arm64
+    runs-on: windows-latest
+    steps:
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Install NASM
+        uses: ilammy/[email protected]
+      - name: Remove wrong clang-cl.exe
+        run:  rm "C:/Program Files/LLVM/bin/clang-cl.exe"
+      - name: Checkout
+        uses: actions/checkout@v4
+      - uses: TheMrMilchmann/setup-msvc-dev@v3
+        with:
+          arch: ${{ matrix.target }}
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Setup CMake
+        uses: threeal/[email protected]
+        with:
+          generator: Ninja
+          c-compiler: clang-cl
+          cxx-compiler: clang-cl
+          options: |
+            CMAKE_CROSSCOMPILING=${{ ((matrix.target  == 'x64') && '0') || '1' }} \
+            CMAKE_SYSTEM_NAME=Windows \
+            CMAKE_SYSTEM_PROCESSOR=x86_64 \
+            CMAKE_BUILD_TYPE=Release \
+      - if: ${{ matrix.target  == 'x64_arm64' }}
+        name: Setup CMake
+        uses: threeal/[email protected]
+        with:
+          generator: Ninja
+          c-compiler: clang-cl
+          cxx-compiler: clang-cl
+          options: |
+            CMAKE_CROSSCOMPILING=1 \
+            CMAKE_SYSTEM_NAME=Windows \
+            CMAKE_SYSTEM_PROCESSOR=ARM64 \
+            CMAKE_C_COMPILER_TARGET=arm64-pc-windows-msvc \
+            CMAKE_ASM_COMPILER_TARGET=arm64-pc-windows-msvc \
+            CMAKE_CXX_COMPILER_TARGET=arm64-pc-windows-msvc \
+            CMAKE_BUILD_TYPE=Release \
+      - name: Build Project
+        run: cmake --build ./build --target all
+      - if: ${{ matrix.target  == 'x64' }}
+        name: Run tests
+        run: cmake --build ./build --target run_tests
   cross-mingw:
     if: github.repository_owner == 'aws'
     runs-on: ubuntu-22.04
@@ -95,4 +147,3 @@ jobs:
       - name: x86_64-w64-mingw32 Build/Test
         run:
           ./tests/ci/run_cross_mingw_tests.sh x86_64 w64-mingw32 "-DCMAKE_BUILD_TYPE=Release"
-
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3089697c7..8dace468f 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -347,7 +347,15 @@ if(GCC OR CLANG)
   # TODO(CryptoAlg-759): enable '-Wpedantic' if awslc has to follow c99 spec.
   if(CLANG OR (GCC AND CMAKE_C_COMPILER_VERSION VERSION_GREATER "4.1.3"))
     # GCC 4.1.3 and below do not support all of these flags or they raise false positives.
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Werror")
+    if (MSVC)
+      # clang-cl sets different default warnings than clang. It also treats -Wall
+      # as -Weverything, to match MSVC. Instead -W3 is the alias for -Wall.
+      # See http://llvm.org/viewvc/llvm-project?view=revision&revision=319116
+      set(C_CXX_FLAGS "${C_CXX_FLAGS} -W3 -Wno-unused-parameter -fmsc-version=1900 -Werror")
+      set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-deprecated-declarations")
+    else()
+      set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fvisibility=hidden -Wall -Wextra -Wno-unused-parameter -Werror")
+    endif()
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wunused -Wcomment -Wchar-subscripts -Wuninitialized -Wshadow")
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wwrite-strings -Wformat-security -Wunused-result")
     set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wvla -Wtype-limits")
@@ -359,12 +367,7 @@ if(GCC OR CLANG)
   endif()

   set(C_CXX_FLAGS "${C_CXX_FLAGS} -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers -Wwrite-strings")
-  if(MSVC)
-    # clang-cl sets different default warnings than clang. It also treats -Wall
-    # as -Weverything, to match MSVC. Instead -W3 is the alias for -Wall.
-    # See http://llvm.org/viewvc/llvm-project?view=revision&revision=319116
-    set(C_CXX_FLAGS "${C_CXX_FLAGS} -W3 -Wno-unused-parameter -fmsc-version=1900")
-  else()
+  if(!MSVC)
     if(EMSCRIPTEN)
       # emscripten's emcc/clang does not accept the "-ggdb" flag.
       set(C_CXX_FLAGS "${C_CXX_FLAGS} -g")
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.91%. Comparing base (e82f824) to head (b02504b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1538   +/-   ##
=======================================
  Coverage   77.91%   77.91%           
=======================================
  Files         560      560           
  Lines       94613    94613           
  Branches    13618    13618           
=======================================
+ Hits        73720    73722    +2     
+ Misses      20299    20298    -1     
+ Partials      594      593    -1     

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

@justsmth justsmth enabled auto-merge (squash) May 13, 2024 19:14
@justsmth justsmth merged commit e3c4f7d into aws:main May 13, 2024
81 checks passed
@justsmth justsmth deleted the windows-clang-cl branch May 13, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants