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

feat: windows toolchain for running clang tools #342

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions platforms/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,11 @@ platform(
"@platforms//cpu:aarch64",
],
)

platform(
name = "windows-x86_64",
constraint_values = [
"@platforms//os:windows",
"@platforms//cpu:x86_64",
],
)
170 changes: 170 additions & 0 deletions toolchain/BUILD.llvm_repo_windows
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
# Copyright 2021 The Bazel Authors.
Copy link
Collaborator

@rrbutani rrbutani Jun 17, 2024

Choose a reason for hiding this comment

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

WDYT about unifying this with BUILD.llvm_repo? It seems like the discrepancies are mostly just in the file extensions (.exe, .lib for Windows).

Maybe something like:

is_windows = %{is_windows} # provided by `llvm_repo`
executable_extension = ".exe" if is_windows else ""

filegroup(
  name = "clang",
  srcs = [
    "bin/" + e + executable_extension for e in [
      "clang", "clang++", "clang-cpp", "clang-cl",
    ]
  ],
)

# ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sounds good but I'm not sure how the templating would work, could you elaborate?

#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

package(default_visibility = ["//visibility:public"])

# Some targets may need to directly depend on these files.
exports_files(glob([
"bin/*",
"lib/*",
"include/*",
]))

## LLVM toolchain files

ALL_DLLS = glob(["bin/*.dll"])

filegroup(
name = "clang",
srcs = [
"bin/clang.exe",
"bin/clang++.exe",
"bin/clang-cpp.exe",
peakschris marked this conversation as resolved.
Show resolved Hide resolved
"bin/clang-cl.exe",
] + ALL_DLLS,
)

filegroup(
name = "clang-cl",
srcs = [
"bin/clang-cl.exe",
] + ALL_DLLS,
)

filegroup(
name = "llvm-ml",
srcs = [
"bin/llvm-ml.exe",
] + ALL_DLLS,
)

filegroup(
name = "ld",
# a set of 4 identical binaries?
srcs = ["bin/ld.lld.exe", "bin/ld64.lld.exe", "lld-link.exe", "lld.exe"] + ALL_DLLS,
)

filegroup(
name = "lld-link",
srcs = [
"bin/lld-link.exe",
] + ALL_DLLS,
)

filegroup(
name = "llvm-lib",
srcs = [
"bin/llvm-lib.exe",
] + ALL_DLLS,
)

filegroup(
name = "include",
srcs = glob([
"include/**/c++/**",
"lib/clang/*/include/**",
]),
)

filegroup(
name = "all_includes",
srcs = glob(["include/**"]),
)

filegroup(
name = "bin",
srcs = glob(["bin/**"]),
)
Comment on lines +85 to +88
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not very familiar with Windows but IIUC the binaries require the DLLs in bin/ in the Windows release tarball to function; I think we need to add them to bin and clang, as, ar, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't need the DLLs as I already have them installed via MSVC on our systems. But for others without MSVC this is a good point. Have a look at my fix, as I don't know how these filegroups will be consumed downstream I'm not sure if the DLLs should go in srcs or data or simply allow downstream to consume separately from the dlls filegroup.


filegroup(
name = "lib",
srcs = glob(
[
"lib/**/*.lib",
],
exclude = [
"lib/LLVM*.lib",
"lib/clang*.lib",
"lib/lld*.lib",
],
),
)

filegroup(
name = "dll",
srcs = ALL_DLLS,
)

filegroup(
name = "ar",
srcs = ["bin/llvm-ar.exe"] + ALL_DLLS,
)

filegroup(
name = "as",
srcs = [
"bin/clang.exe",
"bin/llvm-as.exe",
] + ALL_DLLS,
)

filegroup(
name = "nm",
srcs = ["bin/llvm-nm.exe"] + ALL_DLLS,
)

filegroup(
name = "objcopy",
srcs = ["bin/llvm-objcopy.exe"] + ALL_DLLS,
)

filegroup(
name = "objdump",
srcs = ["bin/llvm-objdump.exe"] + ALL_DLLS,
)

filegroup(
name = "profdata",
srcs = ["bin/llvm-profdata.exe"] + ALL_DLLS,
)

filegroup(
name = "dwp",
srcs = ["bin/llvm-dwp.exe"] + ALL_DLLS,
)

filegroup(
name = "ranlib",
srcs = ["bin/llvm-ranlib.exe"] + ALL_DLLS,
)

filegroup(
name = "readelf",
srcs = ["bin/llvm-readelf.exe"] + ALL_DLLS,
)

filegroup(
name = "strip",
srcs = ["bin/llvm-strip.exe"] + ALL_DLLS,
)

filegroup(
name = "symbolizer",
srcs = ["bin/llvm-symbolizer.exe"] + ALL_DLLS,
)

filegroup(
name = "clang-tidy",
srcs = ["bin/clang-tidy.exe"] + ALL_DLLS,
)
8 changes: 8 additions & 0 deletions toolchain/cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ def cc_toolchain_config(
"clang",
"glibc_unknown",
),
"windows-x86_64": (
"clang-x86_64-windows",
"k8",
"msvc",
"clang",
"windows_x86_64",
"windows_x86_64",
),
}[target_os_arch_key]

# Unfiltered compiler flags; these are placed at the end of the command
Expand Down
6 changes: 3 additions & 3 deletions toolchain/internal/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

SUPPORTED_TARGETS = [("linux", "x86_64"), ("linux", "aarch64"), ("darwin", "x86_64"), ("darwin", "aarch64")]
SUPPORTED_TARGETS = [("linux", "x86_64"), ("linux", "aarch64"), ("darwin", "x86_64"), ("darwin", "aarch64"), ("windows", "x86_64")]

# Map of tool name to its symlinked name in the tools directory.
# See tool_paths in toolchain/cc_toolchain_config.bzl.
Expand Down Expand Up @@ -104,7 +104,7 @@ def os(rctx):

name = rctx.attr.exec_os
if name:
if name in ("linux", "darwin"):
if name in ("linux", "darwin", "windows"):
return name
else:
fail("Unsupported value for exec_os: %s" % name)
Expand All @@ -120,7 +120,7 @@ def os(rctx):

def os_bzl(os):
# Return the OS string as used in bazel platform constraints.
return {"darwin": "osx", "linux": "linux"}[os]
return {"darwin": "osx", "linux": "linux", "windows": "windows"}[os]

def arch(rctx):
arch = rctx.attr.exec_arch
Expand Down
15 changes: 10 additions & 5 deletions toolchain/internal/configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ def llvm_config_impl(rctx):
_check_os_arch_keys(rctx.attr.cxx_builtin_include_directories)

os = _os(rctx)
if os == "windows":
_empty_repository(rctx)
return
arch = _arch(rctx)

if not rctx.attr.toolchain_roots:
Expand Down Expand Up @@ -311,6 +308,7 @@ def _cc_toolchain_str(
"darwin-aarch64": "aarch64-apple-macosx",
"linux-aarch64": "aarch64-unknown-linux-gnu",
"linux-x86_64": "x86_64-unknown-linux-gnu",
"windows-x86_64": "x86_64-pc-windows-msvc",
}[target_pair]
cxx_builtin_include_directories = [
toolchain_path_prefix + "include/c++/v1",
Expand Down Expand Up @@ -338,7 +336,7 @@ def _cc_toolchain_str(
_join(sysroot_prefix, "/System/Library/Frameworks"),
])
else:
fail("Unreachable")
pass

cxx_builtin_include_directories.extend(toolchain_info.additional_include_dirs_dict.get(target_pair, []))

Expand Down Expand Up @@ -545,15 +543,21 @@ cc_toolchain(
major_llvm_version = major_llvm_version,
)

def _extension(os):
if os == "windows":
return ".exe"
return ""

def _convenience_targets_str(rctx, use_absolute_paths, llvm_dist_rel_path, llvm_dist_label_prefix, exec_dl_ext):
ext = _extension(_os(rctx))
if use_absolute_paths:
llvm_dist_label_prefix = ":"
filenames = []
for libname in _aliased_libs:
filename = "lib/{}.{}".format(libname, exec_dl_ext)
filenames.append(filename)
for toolname in _aliased_tools:
filename = "bin/{}".format(toolname)
filename = "bin/{}{}".format(toolname, ext)
filenames.append(filename)

for filename in filenames:
Expand All @@ -570,6 +574,7 @@ cc_import(

tool_target_strs = []
for name in _aliased_tools:
name = name + ext
template = """
native_binary(
name = "{name}",
Expand Down
10 changes: 10 additions & 0 deletions toolchain/internal/release_name.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ def _darwin(llvm_version, arch):
)

def _windows(llvm_version, arch):
major_llvm_version = _major_llvm_version(llvm_version)
if major_llvm_version >= 18 and arch.endswith("64"):
arch = "x86_64"
suffix = "pc-windows-msvc"
return "clang+llvm-{llvm_version}-{arch}-{suffix}.tar.xz".format(
llvm_version = llvm_version,
arch = arch,
suffix = suffix,
)

if arch.endswith("64"):
win_arch = "win64"
else:
Expand Down
19 changes: 11 additions & 8 deletions toolchain/internal/repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,17 @@ llvm_config_attrs.update({
def llvm_repo_impl(rctx):
os = _os(rctx)
if os == "windows":
rctx.file("BUILD.bazel", executable = False)
return None

rctx.file(
"BUILD.bazel",
content = rctx.read(Label("//toolchain:BUILD.llvm_repo")),
executable = False,
)
rctx.file(
"BUILD.bazel",
content = rctx.read(Label("//toolchain:BUILD.llvm_repo_windows")),
executable = False,
)
else:
rctx.file(
"BUILD.bazel",
content = rctx.read(Label("//toolchain:BUILD.llvm_repo")),
executable = False,
)

updated_attrs = _download_llvm(rctx)

Expand Down
Loading