Skip to content

Conversation

synthMoza
Copy link

Currently one might invoke llvm-tblgen in Bazel by using gentbl macro. But bazel macros do not allow you to use select statements with them. For example, one might use different tblgen_args on different OS, or depending on command line options. So, in this PR I implemented tblgen as a rule instead of a macro.

tblgen_args = select({
          "@platforms//os:windows": "--long-string-literals=0",
          "//conditions:default": "",
      }),

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Jun 12, 2024
@llvmbot llvmbot added the libc label Jun 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 12, 2024

@llvm/pr-subscribers-libc

Author: Eganyan Alexey (synthMoza)

Changes

Currently one might invoke llvm-tblgen in Bazel by using gentbl macro. But bazel macros do not allow you to use select statements with them. For example, one might use different tblgen_args on different OS, or depending on command line options. So, in this PR I implemented tblgen as a rule instead of a macro.

tblgen_args = select({
          "@<!-- -->platforms//os:windows": "--long-string-literals=0",
          "//conditions:default": "",
      }),

Full diff: https://github.com/llvm/llvm-project/pull/95324.diff

3 Files Affected:

  • (modified) libc/src/__support/fixedvector.h (+11-11)
  • (modified) utils/bazel/llvm-project-overlay/llvm/BUILD.bazel (+5-5)
  • (modified) utils/bazel/llvm-project-overlay/llvm/tblgen.bzl (+50-18)
diff --git a/libc/src/__support/fixedvector.h b/libc/src/__support/fixedvector.h
index ddd0993a95272..43028a0a84637 100644
--- a/libc/src/__support/fixedvector.h
+++ b/libc/src/__support/fixedvector.h
@@ -25,17 +25,17 @@ template <typename T, size_t CAPACITY> class FixedVector {
   constexpr FixedVector() = default;
 
   using iterator = typename cpp::array<T, CAPACITY>::iterator;
-  constexpr FixedVector(iterator begin, iterator end) {
+  constexpr FixedVector(iterator begin, iterator end) : store{}, item_count{} {
     for (; begin != end; ++begin)
       push_back(*begin);
   }
 
-  constexpr FixedVector(size_t count, const T &value) {
+  constexpr FixedVector(size_t count, const T &value) : store{}, item_count{} {
     for (size_t i = 0; i < count; ++i)
       push_back(value);
   }
 
-  bool push_back(const T &obj) {
+  constexpr bool push_back(const T &obj) {
     if (item_count == CAPACITY)
       return false;
     store[item_count] = obj;
@@ -43,27 +43,27 @@ template <typename T, size_t CAPACITY> class FixedVector {
     return true;
   }
 
-  const T &back() const { return store[item_count - 1]; }
+  constexpr const T &back() const { return store[item_count - 1]; }
 
-  T &back() { return store[item_count - 1]; }
+  constexpr T &back() { return store[item_count - 1]; }
 
-  bool pop_back() {
+  constexpr bool pop_back() {
     if (item_count == 0)
       return false;
     --item_count;
     return true;
   }
 
-  T &operator[](size_t idx) { return store[idx]; }
+  constexpr T &operator[](size_t idx) { return store[idx]; }
 
-  const T &operator[](size_t idx) const { return store[idx]; }
+  constexpr const T &operator[](size_t idx) const { return store[idx]; }
 
-  bool empty() const { return item_count == 0; }
+  constexpr bool empty() const { return item_count == 0; }
 
-  size_t size() const { return item_count; }
+  constexpr size_t size() const { return item_count; }
 
   // Empties the store for all practical purposes.
-  void reset() { item_count = 0; }
+  constexpr void reset() { item_count = 0; }
 
   // This static method does not free up the resources held by |store|,
   // say by calling `free` or something similar. It just does the equivalent
diff --git a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
index cf7ee18f0d068..4bd9044a15f0a 100644
--- a/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel
@@ -2343,11 +2343,11 @@ gentbl(
         tblgen = ":llvm-tblgen",
         # MSVC isn't happy with long string literals, while other compilers
         # which support them get significant compile time improvements with
-        # them enabled. Ideally this flag would only be enabled on Windows via
-        # a select() on `@platforms//os:windows,`, but that would
-        # require refactoring gentbl from a macro into a rule.
-        # TODO(#92): Refactor gentbl to support this use
-        tblgen_args = "--long-string-literals=0",
+        # them enabled.
+        tblgen_args = select({
+            "@platforms//os:windows": "--long-string-literals=0",
+            "//conditions:default": "",
+        }),
         td_file = "lib/Target/" + target["name"] + "/" + target["short_name"] + ".td",
         td_srcs = [
             ":common_target_td_sources",
diff --git a/utils/bazel/llvm-project-overlay/llvm/tblgen.bzl b/utils/bazel/llvm-project-overlay/llvm/tblgen.bzl
index d43390918e390..eebf6ea38e51f 100644
--- a/utils/bazel/llvm-project-overlay/llvm/tblgen.bzl
+++ b/utils/bazel/llvm-project-overlay/llvm/tblgen.bzl
@@ -41,24 +41,15 @@ def gentbl(
         td_srcs += [td_file]
     for (opts, out) in tbl_outs:
         rule_suffix = "_".join(opts.replace("-", "_").replace("=", "_").split(" "))
-        native.genrule(
-            name = "%s_%s_genrule" % (name, rule_suffix),
-            srcs = td_srcs,
-            outs = [out],
-            tools = [tblgen],
-            message = "Generating code from table: %s" % td_file,
-            cmd = (("$(location %s) -I %s/llvm/include " +
-                    "-I %s/clang/include " +
-                    "-I $$(dirname $(location %s)) " +
-                    "%s $(location %s) %s -o $@") % (
-                tblgen,
-                llvm_project_execroot_path,
-                llvm_project_execroot_path,
-                td_file,
-                opts,
-                td_file,
-                tblgen_args,
-            )),
+        _gentbl(
+            name = "%s_%s_rule" % (name, rule_suffix),
+            tblgen = tblgen,
+            td_file = td_file,
+            td_srcs = td_srcs,
+            opts = opts,
+            out = out,
+            tblgen_args = tblgen_args,
+            llvm_project_execroot_path = llvm_project_execroot_path,
         )
 
     # For now, all generated files can be assumed to comprise public interfaces.
@@ -79,3 +70,44 @@ def gentbl(
             features = ["-parse_headers", "-header_modules"],
             **kwargs
         )
+
+def _gentbl_impl(ctx):
+    inputs = depset(ctx.files.td_srcs)
+
+    args = ctx.actions.args()
+    args.add("-I", "%s/llvm/include" % ctx.attr.llvm_project_execroot_path)
+    args.add("-I", "%s/clang/include" % ctx.attr.llvm_project_execroot_path)
+    args.add("-I", ctx.file.td_file.dirname)
+    
+    parsed_opts = ctx.attr.opts.split(' ')
+    for opt in parsed_opts:
+        args.add(opt)
+
+    if ctx.attr.tblgen_args:
+        parsed_args = ctx.attr.tblgen_args.split(' ')
+        for tblgen_arg in parsed_args:
+            args.add(tblgen_arg)
+
+    args.add(ctx.file.td_file)
+    args.add("-o", ctx.outputs.out)
+
+    ctx.actions.run(
+        mnemonic = "tblgen",
+        executable = ctx.executable.tblgen,
+        arguments = [args],
+        inputs = inputs,
+        outputs = [ctx.outputs.out],
+    )
+
+_gentbl = rule(
+    implementation = _gentbl_impl,
+    attrs = {
+        "tblgen": attr.label(executable = True, cfg = "exec", doc = "The binary used to produce the output."),
+        "td_file": attr.label(allow_single_file = True, doc = "The binary used to produce the output"),
+        "td_srcs": attr.label_list(allow_files = True, doc = "A list of table definition files included transitively."),
+        "opts": attr.string(doc = "String of options passed to tblgen."),
+        "out": attr.output(doc = "Corresponding to opts output file."),
+        "llvm_project_execroot_path": attr.string(doc = "Path to llvm-project execroot."),
+        "tblgen_args": attr.string(default = "", doc = "Extra arguments string to pass to the tblgen binary."),
+    }
+)

@synthMoza
Copy link
Author

Ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants