[Bazel] Switch LLVM targets based on configuration flags.

[izuk]

Summary

Added a flag @llvm-project//llvm:targets to control which LLVM targets are
built at compile time:

$ bazel build --@llvm-projectllvm:targets=AArch64,X86 --config=generic_clang @llvm-project...

[GMNGeoffrey]

I think you need to rebase for the build to pass

[izuk]

Rebased off HEAD.

[GMNGeoffrey]

This overall seems like an improvement. Some thoughts:

1. Being able to just list the targets you want seems good. Do we also have a use case for "everything except target X" though? That would be hard to express in this scheme, whereas with individual target enable/disable flags it would be easy. Is there a way to get both? Is it worth it?
2. It would be nice to be able to have targets that are configured, but are disabled by default. I guess we can add that pretty easily by creating a separate variable and using that for build_setting_default in the string list flag.
3. It's a bit hard to guarantee that nothing directly references the targets. I think we do want it to be possible to directly reference a specific target, but if someone passes a flag without a target enabled, it seems like we shouldn't build that target. As I suggest above, perhaps we should make the srcs of disabled targets empty.
4. As a test, can you try building with no targets enabled? I'm curious what happens. It might also flush out edge cases. What about with only some weird target enabled (and no X86)

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

29–30

Can you include an example usage, as you have in the patch description here?

68–69

When I first read this I thought you were talking about Starlark macros. Maybe that could be clarified?

1781

Perhaps we should select out all srcs if the target isn't enabled. Then there's no chance some other dependency is going to pull in anything from a disabled target

utils/bazel/llvm-project-overlay/llvm/enum_targets_gen.bzl

5

Please update the documentation

63

I had to read the code to understand what this meant. Can you expand the documentation a bit? It is probably also worth explaining *why* you need two parameters here rather than just passing the targets you want (because this is dependent on selects and therefore has to be processed within a rule).

utils/bazel/llvm-project-overlay/llvm/target_macros.bzl

35–38

Just seems a bit cleaner

[izuk]

Addressed documentation comments.
Changed enum_targets_gen into a macro.

[izuk]

I tried to address all the documentation comments.

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

1781

Not sure I understand. Do you mean, have the Bazel target but with an empty srcs?

[GMNGeoffrey]

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

1781

Yes, exactly. The set of targets has to be static, but we could make the Bazel target a noop, so that disabling an LLVM target really guarantees it isn't built.

utils/bazel/llvm-project-overlay/llvm/enum_targets_gen.bzl

53

Does that work? 🤨 Selects aren't evaluated within macros (which is why this rule exists). I think you're going to end up with just "if <object>" which will always be True

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

 # It may be tempting to add compiler flags here, but that should be avoided.
 # The necessary warnings and other compile flags should be provided by the
 # toolchain or the `.bazelrc` file. This is just a workaround until we have a
 # widely available feature to enable unlimited stack frame instead of using
 # this `Make` variable.
 llvm_copts = [
     "$(STACK_FRAME_UNLIMITED)",
 ]
 
 # A command-line flag for restricting the list of LLVM targets to build.
 #
 # See `llvm_targets` in `:targets.bzl` for the full list of available targets.
 #
 # Usage:
 #
 # $ bazel build --@llvm-projectllvm:targets=AArch64,X86 --config=generic_clang @llvm-project...
 string_list_flag(
     name = "targets",

 # Expand LLVM target enumerations in C++ headers with lists of enabled targets.
 [
     enum_targets_gen(
         name = name,
         src = "include/llvm/Config/{}.def.in".format(path),
         out = "include/llvm/Config/{}.def".format(path),
         macro_name = macro_name,
         valid_targets = targets_with_file(pattern),
-        targets = ":targets",
     ) for name, macro_name, path, pattern in [
         ("targets_def_gen", "TARGET", "Targets", "CMakeLists.txt"),
         ("asm_printers_def_gen", "ASM_PRINTER", "AsmPrinters", "*AsmPrinter.cpp"),
         ("asm_parsers_def_gen", "ASM_PARSER", "AsmParsers", "AsmParser/CMakeLists.txt"),
         ("disassemblers_def_gen", "DISASSEMBLER", "Disassemblers", "Disassembler/CMakeLists.txt"),
         ("target_mca_def_gen", "TARGETMCA", "TargetMCAs", "MCA/CMakeLists.txt"),
     ]
 ]
 
 # TODO: Need to replace this with something that actually extracts the git
 # commit from the LLVM source (submodule or http_archive).
 genrule(
     name = "version_info_gen",
     outs = ["include/llvm/Config/VersionInfo.h"],
     cmd = "echo '#define LLVM_VERSION_INFO \"git\"' > $@",
 )
 
 template_rule(
[gentbl(
         ] + glob([
             "lib/Target/" + target["name"] + "/*.td",
             "lib/Target/" + target["name"] + "/GISel/*.td",
         ]),
         deps = target.get("tbl_deps", []),
     )],
     [cc_library(
         name = target["name"] + "Info",
         srcs = ["lib/Target/" + target["name"] + "/TargetInfo/" + target["name"] + "TargetInfo.cpp"],
         hdrs = glob(["lib/Target/" + target["name"] + "/TargetInfo/*.h"]),
         copts = llvm_copts,
         # Workaround for https://github.com/bazelbuild/bazel/issues/3828
         # TODO(gcmn): Remove this when upgrading to a Bazel version containing
         # https://github.com/bazelbuild/bazel/commit/e3b7e17b05f1
         includes = ["lib/Target/" + target["name"]],
         strip_include_prefix = "lib/Target/" + target["name"],
         deps = [

utils/bazel/llvm-project-overlay/llvm/enum_targets_gen.bzl

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 """A rule to expand LLVM target enumerations.
 
 Replaces in a text file a single variable of the style `@LLVM_ENUM_FOOS@` with a
 list of macro invocations, one for each target on its own line:
 
 ```
 LLVM_FOO(TARGET1)
 LLVM_FOO(TARGET2)
 // ...
 ```
 
 Example:
 load(":enum_targets_gen.bzl", "enum_targets_gen")
 
 enum_targets_gen(
     name = "disassemblers_def_gen",
     src = "include/llvm/Config/Disassemblers.def.in",
     out = "include/llvm/Config/Disassemblers.def",
     macro_name = "DISASSEMBLER",
     valid_targets = ["AArch64", "X86"],
-    targets = ":targets",
 )
 
 This rule provides a slightly more semantic API than template_rule, but the main
 reason it exists is to permit a list with selects to be passed for `targets` as
 a select is not allowed to be passed to a rule within another data structure. 
 """
 
-load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo")
+load(":expand_template.bzl", "expand_template")
+load(":target_macros.bzl", "is_enabled")
 
-def _enum_targets_gen_impl(ctx):
-    to_replace = "@LLVM_ENUM_{}S@".format(ctx.attr.macro_name)
+def enum_targets_gen(name, src, out, macro_name, valid_targets):
+    """Replaces @LLVM_ENUM_FOOS@ with a list of macro invocations.
+
+    Args:
+      name: The name of the generated BUILD target.
+      src: The input file to expand.
+      out: The output file to generate.
+      macro_name: The name of the enumeration. This is the suffix of the
+          placeholder being replaced `@LLVM_ENUM_{}S@` and of the
+          macro invocations generated `LLVM_{}(TARGET)`. Should be
+          all caps and singular, e.g. 'DISASSEMBLER'.
+      valid_targets: A list of which LLVM targets are valid for expansion.
+          Only enabled LLVM targets are actually expanded.
+    """
+    to_replace = "@LLVM_ENUM_{}S@".format(macro_name)
     replacement = "\n".join([
-        "LLVM_{}({})".format(ctx.attr.macro_name, t)
-        for t in ctx.attr.targets[BuildSettingInfo].value
-        if t in ctx.attr.valid_targets
+        "LLVM_{}({})".format(macro_name, t)
+        for t in valid_targets
+        if is_enabled(t)
     ])
-    ctx.actions.expand_template(
-        template = ctx.file.src,
-        output = ctx.outputs.out,
+    expand_template(
+        name = name,
+        src = src,
+        out = out,
         substitutions = {to_replace: replacement},
     )
-
-enum_targets_gen = rule(
-    implementation = _enum_targets_gen_impl,
-    attrs = {
-        "src": attr.label(
-            mandatory = True,
-            allow_single_file = True,
-        ),
-        "valid_targets": attr.string_list(
-            mandatory = True,
-            doc = "A static list of LLVM targets that are valid for this enum. " +
-                  "Since this value depends on what files are present, it " +
-                  "must be evaluated outside the implementation of the rule.",
-        ),
-        "targets": attr.label(
-            mandatory = True,
-            doc = "A configuration of LLVM targets to build.",
-        ),
-        "out": attr.output(mandatory = True),
-        "macro_name": attr.string(
-            mandatory = True,
-            doc = "The name of the enumeration. This is the suffix of the" +
-                  " placeholder being replaced `@LLVM_ENUM_{}S@` and of the" +
-                  " macro invocations generated `LLVM_{}(TARGET)`. Should be" +
-                  " all caps and singular, e.g. 'DISASSEMBLER'",
-        ),
-    },
-    # output_to_genfiles is required for header files.
-    output_to_genfiles = True,
-)

utils/bazel/llvm-project-overlay/llvm/expand_template.bzl

+# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+"""A rule to expand templates with substitutions.
+
+This is a wrapper around `actions.expand_template` so that we can use it
+from within macros.
+"""
+
+def _expand_template_impl(ctx):
+    ctx.actions.expand_template(
+        template = ctx.file.src,
+        output = ctx.outputs.out,
+        substitutions = ctx.attr.substitutions,
+    )
+    return DefaultInfo(files = depset([ctx.outputs.out]))
+
+expand_template = rule(
+    implementation = _expand_template_impl,
+    attrs = {
+        "src": attr.label(
+            mandatory = True,
+            allow_single_file = True,
+        ),
+        "out": attr.output(mandatory = True),
+        "substitutions": attr.string_dict(),
+    },
+    output_to_genfiles = True,
+)

utils/bazel/llvm-project-overlay/llvm/target_macros.bzl

def targets_with_file(suffix):
 
     The sources for LLVM targets are in folders `lib/Target/*`.  This macro
     finds all targets that contain a file matching the suffix, ie.,
     `lib/Target/<target>/<suffix>`.
     """
     matches = native.glob(["lib/Target/*/{}".format(suffix)])
     return [m.split("/")[2] for m in matches]
 
+def is_enabled(target):
+    return select({
+          ":{}_enabled".format(target): True,
+          "//conditions:default": False,
+        })
+
 def enabled_targets(pattern):
     """Build a list of enabled LLVM targets.
 
     This macro builds a depency list that evaluates to Bazel targets of the
     form `<target><pattern>` for every enabled LLVM target.
     """
     # Build a list of selects that either choose the Bazel target or not,
     # depending on the `:targets` flag.
     selects = [
         select({
-            ":%s_enabled" % t: ["{}{}".format(t, pattern)],
+            ":{}_enabled".format(t): ["{}{}".format(t, pattern)],
             "//conditions:default": [],
         })
         for t in llvm_targets
     ]
     # Fold over the selects with "+".
     result = []
     for s in selects:
       result += s
     return result