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

Add support for Visual Studio 2022 #1177

Merged
merged 3 commits into from
Jan 28, 2023
Merged

Add support for Visual Studio 2022 #1177

merged 3 commits into from
Jan 28, 2023

Conversation

juj
Copy link
Collaborator

@juj juj commented Jan 27, 2023

This PR does the following:

  1. Add support for building Emscripten with Visual Studio 2022
  2. Remove some code duplication in different places around VS version selection
  3. Migrate to using cmake --build also when building on Windows. Historically we didn't use cmake --build because it was unable to find where MSBuild.exe resided, but it looks like those problems are long gone.
  4. To enable cmake --build on Windows, leverage the MSBuild 'Multi-ToolTask' feature CL_MPCount to saturate project builds properly to 100% CPU usage so building LLVM builds different cpp files in parallel.

juj added 2 commits January 27, 2023 19:17
… when building on Windows. Leverage the VS2019 MSBuild 'Multi-ToolTask' feature CL_MPCount to saturate project builds properly to 100% CPU usage so building LLVM builds different cpp files in parallel. Clean up some code duplication around Visual Studio support.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

In the long run, is there any reason not to simply require ninja for performing these builds?

@juj juj enabled auto-merge (squash) January 27, 2023 17:29
@juj
Copy link
Collaborator Author

juj commented Jan 27, 2023

In the long run, is there any reason not to simply require ninja for performing these builds?

If we embed Ninja precompiled into the repository, then maybe not. Though assuming upstream CMake fixes the CL_MPCount flags out of the box one day, doing cmake followed by cmake --build . should be clean enough.

@sbc100
Copy link
Collaborator

sbc100 commented Jan 27, 2023

Looks like the version of cmake we have on the bots doesn't support --build + -j:

Running build: ['cmake', '--build', '.', '--config', 'Release', '-j', '4']
Unknown argument -j
Unknown argument 4

@sbc100
Copy link
Collaborator

sbc100 commented Jan 27, 2023

I think maybe you need -- between the cmake flags and the build tool flags (like -j)

@juj
Copy link
Collaborator Author

juj commented Jan 27, 2023

Huh, Linux test fails on the cmake --build -j4 command line. It states

Running build: ['cmake', '--build', '.', '--config', 'Release', '-j', '4']
Unknown argument -j
Unknown argument 4
Usage: cmake --build <dir> [options] [-- [native-options]]
Options:
  <dir>          = Project binary directory to be built.
  --target <tgt> = Build <tgt> instead of default targets.
                   May only be specified once.
  --config <cfg> = For multi-configuration tools, choose <cfg>.
  --clean-first  = Build target 'clean' first, then build.
                   (To clean only, use --target 'clean'.)
  --use-stderr   = Ignored.  Behavior is default in CMake >= 3.0.
  --             = Pass remaining options to the native tool.
Build failed due to exception!
Working directory: /root/project/binaryen/main_64bit_binaryen
Command '['cmake', '--build', '.', '--config', 'Release', '-j', '4']' returned non-zero exit status 1.
error: installation failed!

When I run cmake --build locally, I get

C:\emsdk>cmake --build
Usage: cmake --build <dir>             [options] [-- [native-options]]
       cmake --build --preset <preset> [options] [-- [native-options]]
Options:
  <dir>          = Project binary directory to be built.
  --preset <preset>, --preset=<preset>
                 = Specify a build preset.
  --list-presets[=<type>]
                 = List available build presets.
  --parallel [<jobs>], -j [<jobs>]
                 = Build in parallel using the given number of jobs.
                   If <jobs> is omitted the native build tool's
                   default number is used.
                   The CMAKE_BUILD_PARALLEL_LEVEL environment variable
                   specifies a default parallel level when this option
                   is not given.
  -t <tgt>..., --target <tgt>...
                 = Build <tgt> instead of default targets.
  --config <cfg> = For multi-configuration tools, choose <cfg>.
  --clean-first  = Build target 'clean' first, then build.
                   (To clean only, use --target 'clean'.)
  --resolve-package-references={on|only|off}
                 = Restore/resolve package references during build.
  -v, --verbose  = Enable verbose output - if supported - including
                   the build commands to be executed.
  --             = Pass remaining options to the native tool.

C:\emsdk>cmake --version
cmake version 3.25.1

According to https://github.com/Kitware/CMake/blob/master/Help/manual/cmake.1.rst -j flag would have been added in CMake 3.12, which was released on 18 Jul 2018. Building LLVM requires CMake 3.13.4 at minimum.. I can't quite understand how it is possible that the Linux cmake would not support the cmake --build . -j4 option.

I'll work around by passing the build specific cmake --build . -- -j4 option, but the above doesn't make much sense to me..

emsdk.py Outdated Show resolved Hide resolved
@sbc100
Copy link
Collaborator

sbc100 commented Jan 27, 2023

Looks like the CI bots run ubuntu/bionic which has cmake 3.10.2.

Get:25 http://archive.ubuntu.com/ubuntu bionic-updates/main amd64 cmake amd64 3.10.2-1ubuntu2.18.04.2 [3152 kB]

We don't test building llvm from source in CI, only building binaryen, which I guess doesn't require such a new version of cmake.

Leaving the -- for now seems reasonable.

@juj
Copy link
Collaborator Author

juj commented Jan 27, 2023

Ohhh, that explains..

@juj juj disabled auto-merge January 28, 2023 11:07
@juj
Copy link
Collaborator Author

juj commented Jan 28, 2023

All the tests passed, but somehow the checks are all doubled, with a second copy waiting for status to be reported, and the first copy succeeding?

I'll force-land this, looks like it's all green, but something might be sideways with the CI?

@juj juj merged commit fa5f5f3 into main Jan 28, 2023
@juj juj deleted the vs2022_support branch January 28, 2023 11:08
shlomif pushed a commit to shlomif/emsdk that referenced this pull request Sep 29, 2023
* Add support for Visual Studio 2022 and migrate to using cmake --build when building on Windows. Leverage the VS2019 MSBuild 'Multi-ToolTask' feature CL_MPCount to saturate project builds properly to 100% CPU usage so building LLVM builds different cpp files in parallel. Clean up some code duplication around Visual Studio support.

* flake

* Work around Linux bot not having 'cmake --build . -j' flag.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants