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

CI: prepare CI for ccache updates for MSVC/Windows #2120

Merged
merged 1 commit into from
May 12, 2023

Conversation

ashay
Copy link
Collaborator

@ashay ashay commented May 12, 2023

This patch, by itself, doesn't fix caching on Windows, but once a new
release of ccache is available, caching for Windows builds should start
working again (validated by building ccache from source and using it
with LLVM builds).

Ccache rejects caching when either the /Zi or /ZI flags are used
during compilation on Windows, since these flags tell the compiler to
embed debug information in a PDB file (separate from the object file
produced by the compiler). In particular, our CI builds add the /Zi
flag, making ccache mark these compiler invocations as uncacheable.

But what caused our CI to add debug flags, especially when we specified
-DCMAKE_BUILD_TYPE=Release? On Windows, unless we specify the
--config Release flag during the CMake build step, CMake assumes a
debug build. So all this while, we had been producing debug builds of
torch-mlir for every PR! No doubt it took so long to build the Windows
binaries.

The reason for having to specify the configuration during the build
step (as opposed to the configure step) of CMake on Windows is that
CMake's Visual Studio generators will produce both Release and Debug
profiles during the CMake configure step (thus requiring a build-time
value that tells CMake whether to build in Release or Debug mode).
Luckily, on Linux and macOS, the --config flag seems to be simply
ignored, instead of causing build errors.

Strangely, based on cursory tests, it seems like on Windows we need to
specify the Relase configuration as both -DCMAKE_BUILD_TYPE=Release as
well as --config Release. Dropping either made my build switch to a
Debug configuration.

Additionally, there is a bug in ccache v4.8 (although this is addressed
in trunk) that causes ccache to reject caching if the compiler
invocation includes any flag that starts with /Z, including /Zc,
which is added by LLVM's HandleLLVMOptions.cmake and which isn't related
to debug info or PDB files. The next release of ccache should include
the fix, which is to reject caching only for /Zi and /ZI flags and
not all flags that start with /Z.

As a side note, debugging this problem was possible because of ccache's
log file, which is enabled by: ccache --set-config="log_file=log.txt".


cc: @makslevental

This patch, by itself, doesn't fix caching on Windows, but once a new
release of ccache is available, caching for Windows builds should start
working again (validated by building ccache from source and using it
with LLVM builds).

Ccache rejects caching when either the `/Zi` or `/ZI` flags are used
during compilation on Windows, since these flags tell the compiler to
embed debug information in a PDB file (separate from the object file
produced by the compiler).  In particular, our CI builds add the `/Zi`
flag, making ccache mark these compiler invocations as uncacheable.

But what caused our CI to add debug flags, especially when we specified
`-DCMAKE_BUILD_TYPE=Release`?  On Windows, unless we specify the
`--config Release` flag during the CMake build step, CMake assumes a
debug build.  So all this while, we had been producing debug builds of
torch-mlir for every PR!  No doubt it took so long to build the Windows
binaries.

The reason for having to specify the configuration during the _build_
step (as opposed to the _configure_ step) of CMake on Windows is that
CMake's Visual Studio generators will produce _both_ Release and Debug
profiles during the CMake configure step (thus requiring a build-time
value that tells CMake whether to build in Release or Debug mode).
Luckily, on Linux and macOS, the `--config` flag seems to be simply
ignored, instead of causing build errors.

Strangely, based on cursory tests, it seems like on Windows we need to
specify the Relase configuration as both `-DCMAKE_BUILD_TYPE=Release` as
well as `--config Release`.  Dropping either made my build switch to a
Debug configuration.

Additionally, there is a bug in ccache v4.8 (although this is addressed
in trunk) that causes ccache to reject caching if the compiler
invocation includes any flag that starts with `/Z`, including /`Zc`,
which is added by LLVM's HandleLLVMOptions.cmake and which isn't related
to debug info or PDB files.  The next release of ccache should include
the fix, which is to reject caching only for `/Zi` and `/ZI` flags and
not all flags that start with `/Z`.

As a side note, debugging this problem was possible because of ccache's
log file, which is enabled by: `ccache --set-config="log_file=log.txt"`.
@ashay ashay requested a review from powderluv May 12, 2023 14:40
@makslevental
Copy link
Collaborator

Bruh lol I can tell this was one those "wtf is going on this is against the laws of physics" type bugs (where you have the confluence of multiple wtfs). Congrats on figuring it out.

@ashay
Copy link
Collaborator Author

ashay commented May 12, 2023

Thanks! It did make me go down a rabbit hole, but I feel better about my grasp of the situation now. Can't wait until the next ccache version is released, so that we can have fast builds again!

Copy link
Collaborator

@powderluv powderluv left a comment

Choose a reason for hiding this comment

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

Awesome. We can host a prebuilt cache meanwhile if we want

@ashay
Copy link
Collaborator Author

ashay commented May 12, 2023

We can host a prebuilt cache meanwhile if we want

@powderluv I presume you're referring to the Windows ccache binary. What would be a good place to host it? It would sure beat having to wait three hours before merging PRs, but I wonder if anyone would have objections to fetching binaries from non-authoritative sources.

@ashay ashay merged commit 28bb866 into main May 12, 2023
@ashay ashay deleted the ashay/fix-build-caching-on-windows branch May 12, 2023 17:45
gpetters94 pushed a commit to gpetters94/mlir-npcomp that referenced this pull request Jul 7, 2023
This patch, by itself, doesn't fix caching on Windows, but once a new
release of ccache is available, caching for Windows builds should start
working again (validated by building ccache from source and using it
with LLVM builds).

Ccache rejects caching when either the `/Zi` or `/ZI` flags are used
during compilation on Windows, since these flags tell the compiler to
embed debug information in a PDB file (separate from the object file
produced by the compiler).  In particular, our CI builds add the `/Zi`
flag, making ccache mark these compiler invocations as uncacheable.

But what caused our CI to add debug flags, especially when we specified
`-DCMAKE_BUILD_TYPE=Release`?  On Windows, unless we specify the
`--config Release` flag during the CMake build step, CMake assumes a
debug build.  So all this while, we had been producing debug builds of
torch-mlir for every PR!  No doubt it took so long to build the Windows
binaries.

The reason for having to specify the configuration during the _build_
step (as opposed to the _configure_ step) of CMake on Windows is that
CMake's Visual Studio generators will produce _both_ Release and Debug
profiles during the CMake configure step (thus requiring a build-time
value that tells CMake whether to build in Release or Debug mode).
Luckily, on Linux and macOS, the `--config` flag seems to be simply
ignored, instead of causing build errors.

Strangely, based on cursory tests, it seems like on Windows we need to
specify the Relase configuration as both `-DCMAKE_BUILD_TYPE=Release` as
well as `--config Release`.  Dropping either made my build switch to a
Debug configuration.

Additionally, there is a bug in ccache v4.8 (although this is addressed
in trunk) that causes ccache to reject caching if the compiler
invocation includes any flag that starts with `/Z`, including /`Zc`,
which is added by LLVM's HandleLLVMOptions.cmake and which isn't related
to debug info or PDB files.  The next release of ccache should include
the fix, which is to reject caching only for `/Zi` and `/ZI` flags and
not all flags that start with `/Z`.

As a side note, debugging this problem was possible because of ccache's
log file, which is enabled by: `ccache --set-config="log_file=log.txt"`.
gpetters94 pushed a commit to gpetters94/mlir-npcomp that referenced this pull request Jul 7, 2023
This patch, by itself, doesn't fix caching on Windows, but once a new
release of ccache is available, caching for Windows builds should start
working again (validated by building ccache from source and using it
with LLVM builds).

Ccache rejects caching when either the `/Zi` or `/ZI` flags are used
during compilation on Windows, since these flags tell the compiler to
embed debug information in a PDB file (separate from the object file
produced by the compiler).  In particular, our CI builds add the `/Zi`
flag, making ccache mark these compiler invocations as uncacheable.

But what caused our CI to add debug flags, especially when we specified
`-DCMAKE_BUILD_TYPE=Release`?  On Windows, unless we specify the
`--config Release` flag during the CMake build step, CMake assumes a
debug build.  So all this while, we had been producing debug builds of
torch-mlir for every PR!  No doubt it took so long to build the Windows
binaries.

The reason for having to specify the configuration during the _build_
step (as opposed to the _configure_ step) of CMake on Windows is that
CMake's Visual Studio generators will produce _both_ Release and Debug
profiles during the CMake configure step (thus requiring a build-time
value that tells CMake whether to build in Release or Debug mode).
Luckily, on Linux and macOS, the `--config` flag seems to be simply
ignored, instead of causing build errors.

Strangely, based on cursory tests, it seems like on Windows we need to
specify the Relase configuration as both `-DCMAKE_BUILD_TYPE=Release` as
well as `--config Release`.  Dropping either made my build switch to a
Debug configuration.

Additionally, there is a bug in ccache v4.8 (although this is addressed
in trunk) that causes ccache to reject caching if the compiler
invocation includes any flag that starts with `/Z`, including /`Zc`,
which is added by LLVM's HandleLLVMOptions.cmake and which isn't related
to debug info or PDB files.  The next release of ccache should include
the fix, which is to reject caching only for `/Zi` and `/ZI` flags and
not all flags that start with `/Z`.

As a side note, debugging this problem was possible because of ccache's
log file, which is enabled by: `ccache --set-config="log_file=log.txt"`.
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.

3 participants