This is an archive of the discontinued LLVM Phabricator instance.

[gn build] Add template for running llvm-tblgen and use it to add build file for llvm/lib/IR.
ClosedPublic

Authored by thakis on Nov 28 2018, 2:09 PM.

Details

Summary

Also adds a boring build file for llvm/lib/BinaryFormat (needed by llvm/lib/IR).

lib/IR marks Attributes and IntrinsicsEnum as public_deps (because IR's public headers include the generated .inc files), so projects depending on lib/IR will implicitly depend on them being generated. As a consequence, most targets won't have to explicitly list a dependency on these tablegen steps (contrast with intrinsics_gen in the cmake build).

This doesn't yet have the optimization where tablegen's output is only updated if it's changed.

Diff Detail

Repository
rL LLVM

Event Timeline

thakis created this revision.Nov 28 2018, 2:09 PM
thakis updated this revision to Diff 175770.Nov 28 2018, 2:12 PM

git ls-files '*.gn' '*.gni' | xargs -n 1 gn format

thakis edited the summary of this revision. (Show Details)Nov 28 2018, 2:16 PM

…wait, putting the copy-if-different logic in the tablegen runner as I did doesn't actually work, since tablegen will write the name of the temp file in the depfile this way. I'll just put a FIXME for adding write-if-different back into tablegen itself (aka reland r330742) and revert the runner script to the 2 lines it originally was. I'll upload a new patch set in an hour or two; if you look earlier please ignore run_tablegen.py (the rest will stay as-is).

thakis updated this revision to Diff 175781.Nov 28 2018, 3:41 PM
thakis edited the summary of this revision. (Show Details)

remove not functional "write if different" attempt

phosek added inline comments.Nov 28 2018, 11:37 PM
llvm/utils/gn/build/run_tablegen.py
8 ↗(On Diff #175781)

Chromium version does more fancy error checking on Windows, shall we do the same?

llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
27 ↗(On Diff #175781)

Have you considered copying Chromium's [compiled_action.gni](https://chromium.googlesource.com/chromium/src/+/master/build/compiled_action.gni) and then implementing tablegen in terms of that? It seems generally useful as I suspect you'll need the same functionality for Clang tablegen as well in the future.

37 ↗(On Diff #175781)

You should probably forward testonly as well`.

thakis marked 3 inline comments as done.Nov 29 2018, 7:42 AM

Thanks for the review!

llvm/utils/gn/build/run_tablegen.py
8 ↗(On Diff #175781)

That's somewhat recent (https://chromium.googlesource.com/chromium/src/+/8c50b3ed049c736406943ac24dc40fc8ac1e782c). Since this script is only used to run tblgen, shorter code seemed preferable over the small utility gain.

llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
27 ↗(On Diff #175781)

I did consider it! Right now this is only used for tablegen though, and for upstreaming I figured I'd make clang_tblgen call this template and make tblgen_target something the invoker can set, and then clang_tblgen would look like (pseudocode):

template("clang_tblgen") {

tablegen($target_name) {
  tblgen_target = "//clang/utils/TableGen:clang-tblgen"
}

}

(in my prototyping branch, clang-tblgen is just a copy-pasted version of this file so I'm not 100% sure if this is going to pan out, but I think it should.)

37 ↗(On Diff #175781)

I haven't needed it yet -- are you aware of any test-only tblgen invocations?

phosek accepted this revision.Nov 29 2018, 2:05 PM

LGTM

llvm/utils/gn/secondary/llvm/utils/TableGen/tablegen.gni
37 ↗(On Diff #175781)

Nope, but it's a general pattern to always forward visibility and testonly.

This revision is now accepted and ready to land.Nov 29 2018, 2:05 PM
This revision was automatically updated to reflect the committed changes.