Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
4 changes: 1 addition & 3 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
# Global options

common

build -c opt \
--incompatible_enable_cc_toolchain_resolution \
--incompatible_require_linker_input_cc_api
--incompatible_require_linker_input_cc_api

# Aliases for user-defined flags
build --flag_alias=backend_config=@config//:backend_config
Expand Down
6 changes: 0 additions & 6 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@ sh_config_ext = use_extension("@onedal//dev/bazel/toolchains:cc_toolchain_extens
use_repo(sh_config_ext, "onedal_cc_toolchain")
register_toolchains("@{}//:all".format("onedal_cc_toolchain"))


extra_toolchain_ext = use_extension("@onedal//dev/bazel/toolchains:extra_toolchain_extension.bzl", "onedal_extra_toolchain_extension")
use_repo(extra_toolchain_ext, "onedal_extra_toolchain")
register_toolchains("@{}//:all".format("onedal_extra_toolchain"))


http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
name = "catch2",
Expand Down
63 changes: 34 additions & 29 deletions dev/bazel/cc.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#===============================================================================

load("@onedal//dev/bazel:utils.bzl",
"utils",
"paths",
"sets",
)
Expand Down Expand Up @@ -317,31 +316,39 @@ cc_static_lib = rule(
)


def _copy_dynamic_release_file(ctx, src, out_name, is_windows = False, extra_inputs = []):
def _copy_windows_release_file(ctx, src, out_name, extra_inputs = []):
"""Copy a linker output under the file name the release layout expects.

Windows only: the Linux release names come straight out of the link action,
while a DLL and its import library have to be renamed afterwards (see
`_cc_dynamic_lib_impl`). A symlink is not enough, because creating one
requires developer mode on Windows.

Args:
ctx: rule context.
src: the file produced by the link action.
out_name: base name of the copy, declared in the current package.
extra_inputs: further link outputs to declare as inputs, so that the
copy cannot run before the whole link action completed.

Returns:
The declared copy.
"""
out = ctx.actions.declare_file(out_name)
if is_windows:
ctx.actions.run(
executable = "cmd.exe",
inputs = [src] + extra_inputs,
outputs = [out],
arguments = [
"/d",
"/c",
'copy /Y "{}" "{}"'.format(
src.path.replace("/", "\\"),
out.path.replace("/", "\\"),
),
],
use_default_shell_env = True,
)
else:
ctx.actions.run(
executable = "cp",
inputs = [src] + extra_inputs,
outputs = [out],
arguments = [src.path, out.path],
use_default_shell_env = True,
)
ctx.actions.run(
executable = "cmd.exe",
inputs = [src] + extra_inputs,
outputs = [out],
arguments = [
"/d",
"/c",
'copy /Y "{}" "{}"'.format(
src.path.replace("/", "\\"),
out.path.replace("/", "\\"),
),
],
use_default_shell_env = True,
)
return out


Expand Down Expand Up @@ -407,19 +414,17 @@ def _cc_dynamic_lib_impl(ctx):
if dynamic_outputs.dynamic_library.basename == dynamic_release_name:
default_files.append(dynamic_outputs.dynamic_library)
else:
default_files.append(_copy_dynamic_release_file(
default_files.append(_copy_windows_release_file(
ctx,
dynamic_outputs.dynamic_library,
dynamic_release_name,
is_windows = is_windows,
extra_inputs = [dynamic_outputs.interface_library] if dynamic_outputs.interface_library else [],
))
if dynamic_outputs.interface_library:
default_files.append(_copy_dynamic_release_file(
default_files.append(_copy_windows_release_file(
ctx,
dynamic_outputs.interface_library,
"{}_dll.lib".format(rt_name),
is_windows = is_windows,
))
default_info = DefaultInfo(
files = depset(default_files),
Expand Down
1 change: 0 additions & 1 deletion dev/bazel/cc/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ load("@rules_cc//cc/common:cc_info.bzl", "CcInfo")

load("@onedal//dev/bazel:utils.bzl",
"utils",
"paths",
"sets",
)

Expand Down
1 change: 0 additions & 1 deletion dev/bazel/cc/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ load("@rules_cc//cc:action_names.bzl", "ACTION_NAMES")
load("@onedal//dev/bazel:utils.bzl",
"utils",
"paths",
"sets",
)

load("@onedal//dev/bazel/toolchains:action_names.bzl",
Expand Down
48 changes: 33 additions & 15 deletions dev/bazel/daal.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,6 @@ daal_generate_version = rule(
},
)

def _get_tool_for_kernel_defines_patching(ctx):
return ctx.toolchains["@onedal//dev/bazel/toolchains:extra"] \
.extra_toolchain_info.patch_daal_kernel_defines

def _get_disabled_cpus(ctx):
cpu_info = ctx.attr._cpus[CpuInfo]
all_cpus = sets.make(cpu_info.allowed)
Expand All @@ -178,17 +174,37 @@ def _declare_patched_kernel_defines(ctx):
return ctx.actions.declare_file(patched_path)

def _daal_patch_kernel_defines_impl(ctx):
disabled_cpus = _get_disabled_cpus(ctx)
"""Comment out the `DAAL_KERNEL_<ISA>` defines of the disabled ISAs.

`cpp/daal/include/services/internal/daal_kernel_defines.h` enables every
CPU dispatch variant oneDAL can build; the ones `--cpu` leaves out have to
be removed from the header before it is compiled or released. The Makefile
does it with

sed -b -i -E -e 's/^#define DAAL_KERNEL_<ISA>\\b/$(sed.eol)/'

where `sed.eol` is empty on Linux and a lone CR on Windows, so that a
disabled define leaves behind an empty line with the platform's own line
ending while every other line is untouched. `expand_template` does exactly
that substitution, without a helper script: it rewrites the file at
execution time, so nothing has to read its contents during analysis.

The substitution is literal, not anchored like sed's `^...\\b`, which is
equivalent here because the header holds one bare `#define DAAL_KERNEL_*`
per line and mentions those macros nowhere else.
"""
disabled_cpus = sets.to_list(_get_disabled_cpus(ctx))
kernel_defines = _declare_patched_kernel_defines(ctx)
ctx.actions.run(
executable = _get_tool_for_kernel_defines_patching(ctx),
arguments = [
ctx.file.src.path,
kernel_defines.path,
" ".join(sets.to_list(disabled_cpus)),
],
inputs = [ctx.file.src],
outputs = [kernel_defines],
is_windows = ctx.target_platform_has_constraint(
ctx.attr._windows_constraint[platform_common.ConstraintValueInfo],
)
ctx.actions.expand_template(
template = ctx.file.src,
output = kernel_defines,
substitutions = {
"#define DAAL_KERNEL_{}".format(cpu.upper()): "\r" if is_windows else ""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a literal, unanchored replace, so it also hits longer macro names and any other mention of the macro. Demonstrated by adding two lines to the header and building with --cpu=sse2:

#define DAAL_KERNEL_AVX2_VNNI                 ->  _VNNI
/* mirrors #define DAAL_KERNEL_AVX512 */      ->  /* mirrors  */

Make's sed 's/^#define DAAL_KERNEL_X\b//' leaves both lines alone. The docstring's "mentions those macros nowhere else" holds today but nothing enforces it; it breaks silently the day someone adds an AVX2_VNNI / AVX512_* style variant.

Anchoring on the whole line keeps it equivalent, e.g. key "\n#define DAAL_KERNEL_{}\n".format(cpu.upper()) -> "\n\r\n" if is_windows else "\n\n" (the header is eol=lf per .gitattributes, and no define sits on line 1).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, fixed in ab76a83: the key is now "\n#define DAAL_KERNEL_<ISA>\n" -> "\n\n" ("\n\r\n" on Windows), and the docstring is updated. I checked it with your two lines added and --cpu=sse2: AVX2 and AVX512 are blanked (adjacent defines still match one after another, since each replacement keeps its trailing newline), and #define DAAL_KERNEL_AVX2_VNNI and the /* mirrors ... */ comment pass through unchanged.

for cpu in disabled_cpus
},
)
return [ DefaultInfo(files=depset([ kernel_defines ])) ]

Expand All @@ -200,6 +216,8 @@ daal_patch_kernel_defines = rule(
"_cpus": attr.label(
default = "@config//:cpu",
),
"_windows_constraint": attr.label(
default = "@platforms//os:windows",
),
},
toolchains = ["@onedal//dev/bazel/toolchains:extra"],
)
4 changes: 0 additions & 4 deletions dev/bazel/release.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,6 @@ def _copy_data(ctx, prefix):
return dst_files

def _copy_to_release_impl(ctx):
extra_toolchain = ctx.toolchains["@onedal//dev/bazel/toolchains:extra"]
prefix = ctx.attr.name + "/daal/latest"
version_info = ctx.attr._version_info[VersionInfo] if ctx.attr._version_info else None
files = []
Expand Down Expand Up @@ -641,9 +640,6 @@ _release = rule(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
toolchains = [
"@onedal//dev/bazel/toolchains:extra"
],
)

def _headers_filter_impl(ctx):
Expand Down
2 changes: 0 additions & 2 deletions dev/bazel/toolchains/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,3 @@
#===============================================================================

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

toolchain_type(name = "extra")
Loading
Loading