gn build: add RISCV target

[thakis]

Awesome, thanks! Just one real question, the last comment below:

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn

30 ↗

(On Diff #199130)

I think this is the first target that has Utils depending on the higher-up tablegen target. What is this needed for?

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn

4 ↗

(On Diff #199130)

Can you do https://reviews.llvm.org/D61859 for this file?

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/Utils/BUILD.gn

11 ↗

(On Diff #199130)

That's a bit unfortunate :/ I think ARM has this too, but it's a bit yucky. It'd be nicer if BaseInfo wouldn't depend on MCTargetDesc's tablegen internals.

llvm/utils/gn/secondary/llvm/lib/Target/targets.gni

26 ↗

(On Diff #199130)

(pass -U9999 to git diff when generating diffs, so that phab doesn't have to say "Context not available.")

44 ↗

(On Diff #199130)

Since nothing currently reads this, consider not having this variable (and giving the else below an empty body). But up to you, doesn't matter much.

[thakis]

Actually, "the first comment below" – phab showed them in a different order in the preview. I meant this comment: https://reviews.llvm.org/D61821#inline-549166

[dlj]

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn

30 ↗

(On Diff #199130)

Oh, you're completely right... it should go in the public_deps of Utils. (Now I see what you mean... that's what AArch64 does, too.)

llvm/utils/gn/secondary/llvm/lib/Target/targets.gni

26 ↗

(On Diff #199130)

Oops, sorry about that...

[thakis]

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn

30 ↗

(On Diff #199130)

Thanks for moving RISCVGenSystemOperands to Utils/BUILD.gn. Does Utils still need depend on this tablegen group() here? If so, why? (No other target currently has this dep, as far as I can tell.) If not, can you remove the dep and make LLVMRISCVCodeGen dep on the tablegen targets in this file directly (like in other targets)?

[dlj]

llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn

30 ↗

(On Diff #199130)

Ah, yes I missed that. The dep isn't needed in Utils... however, it was hiding a couple of other missing deps (from AsmParser and MCTargetDesc).

[dlj]

Run gn format

[thakis]

Sorry for the long wait here, I wanted to patch this in and look at it some. I've finally gotten around to this.

If it's ok, I'd like to request two changes:

1. (minor) I think LLVMRISCVCodeGen needs a dep on Utils
2. The reason the RISCV:tablegen target is needed is because RISCVGenCompressInstEmitter is specific to RISCV – the rest is like the other targets. So I'd suggest we call out in a comment this target being special, and only give it increased visibility, instead of having a tablegen target.

Full proposed diff:

$ git diff
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/AsmParser/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/AsmParser/BUILD.gn
index 34aba6c3426..ef28b23aca3 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/AsmParser/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/AsmParser/BUILD.gn
@@ -13,7 +13,7 @@ static_library("AsmParser") {
     "//llvm/lib/MC",
     "//llvm/lib/MC/MCParser",
     "//llvm/lib/Support",
-    "//llvm/lib/Target/RISCV:tablegen",
+    "//llvm/lib/Target/RISCV:RISCVGenCompressInstEmitter",
     "//llvm/lib/Target/RISCV/MCTargetDesc",
     "//llvm/lib/Target/RISCV/Utils",
   ]
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn
index 5fff59e36f0..2b9b90935de 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn
@@ -1,45 +1,39 @@
 import("//llvm/utils/TableGen/tablegen.gni")

+# RISCV is the only target that has a "compress instr emitter", and it's
+# a bit strange in that it defines static functions depending on which
+# defines are set. Instead of housing these functions in one library,
+# various libraries include the generated .inc file with different defines set.
 tablegen("RISCVGenCompressInstEmitter") {
-  visibility = [ ":tablegen" ]
+  visibility = [
+    ":LLVMRISCVCodeGen",
+    "AsmParser",
+    "MCTargetDesc",
+  ]
   args = [ "-gen-compress-inst-emitter" ]
   td_file = "RISCV.td"
 }

 tablegen("RISCVGenDAGISel") {
-  visibility = [ ":tablegen" ]
+  visibility = [ ":LLVMRISCVCodeGen" ]
   args = [ "-gen-dag-isel" ]
   td_file = "RISCV.td"
 }

 tablegen("RISCVGenMCPseudoLowering") {
-  visibility = [ ":tablegen" ]
+  visibility = [ ":LLVMRISCVCodeGen" ]
   args = [ "-gen-pseudo-lowering" ]
   td_file = "RISCV.td"
 }

-# This should contain tablegen targets generating .inc files included
-# by other targets. .inc files only used by .cpp files in this directory
-# should be in deps on the static_library instead.
-group("tablegen") {
-  visibility = [
-    ":LLVMRISCVCodeGen",
-    "AsmParser",
-    "MCTargetDesc",
-    "Utils",
-  ]
-  public_deps = [
+static_library("LLVMRISCVCodeGen") {
+  deps = [
     ":RISCVGenCompressInstEmitter",
     ":RISCVGenDAGISel",
     ":RISCVGenMCPseudoLowering",
-  ]
-}
-
-static_library("LLVMRISCVCodeGen") {
-  deps = [
-    ":tablegen",
     "MCTargetDesc",
     "TargetInfo",
+    "Utils",
     "//llvm/include/llvm/Config:llvm-config",
     "//llvm/lib/CodeGen",
     "//llvm/lib/CodeGen/AsmPrinter",
diff --git a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn
index a44b898b635..09ca1808651 100644
--- a/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn
@@ -55,7 +55,7 @@ static_library("MCTargetDesc") {
     ":RISCVGenMCCodeEmitter",
     "//llvm/lib/MC",
     "//llvm/lib/Support",
-    "//llvm/lib/Target/RISCV:tablegen",
+    "//llvm/lib/Target/RISCV:RISCVGenCompressInstEmitter",
     "//llvm/lib/Target/RISCV/Utils",
   ]
   include_dirs = [ ".." ]

Does that sound ok to you? With that, lgtm.

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/AsmParser/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("RISCVGenAsmMatcher") {
+  visibility = [ ":AsmParser" ]
+  args = [ "-gen-asm-matcher" ]
+  td_file = "../RISCV.td"
+}
+
+static_library("AsmParser") {
+  output_name = "LLVMRISCVAsmParser"
+  deps = [
+    ":RISCVGenAsmMatcher",
+    "//llvm/lib/MC",
+    "//llvm/lib/MC/MCParser",
+    "//llvm/lib/Support",
+    "//llvm/lib/Target/RISCV:RISCVGenCompressInstEmitter",
+    "//llvm/lib/Target/RISCV/MCTargetDesc",
+    "//llvm/lib/Target/RISCV/Utils",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVAsmParser.cpp",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+# RISCV is the only target that has a "compress instr emitter", and it's
+# a bit strange in that it defines static functions depending on which
+# defines are set. Instead of housing these functions in one library,
+# various libraries include the generated .inc file with different defines set.
+tablegen("RISCVGenCompressInstEmitter") {
+  visibility = [
+    ":LLVMRISCVCodeGen",
+    "AsmParser",
+    "MCTargetDesc",
+  ]
+  args = [ "-gen-compress-inst-emitter" ]
+  td_file = "RISCV.td"
+}
+
+tablegen("RISCVGenDAGISel") {
+  visibility = [ ":LLVMRISCVCodeGen" ]
+  args = [ "-gen-dag-isel" ]
+  td_file = "RISCV.td"
+}
+
+tablegen("RISCVGenMCPseudoLowering") {
+  visibility = [ ":LLVMRISCVCodeGen" ]
+  args = [ "-gen-pseudo-lowering" ]
+  td_file = "RISCV.td"
+}
+
+static_library("LLVMRISCVCodeGen") {
+  deps = [
+    ":RISCVGenCompressInstEmitter",
+    ":RISCVGenDAGISel",
+    ":RISCVGenMCPseudoLowering",
+    "MCTargetDesc",
+    "TargetInfo",
+    "Utils",
+    "//llvm/include/llvm/Config:llvm-config",
+    "//llvm/lib/CodeGen",
+    "//llvm/lib/CodeGen/AsmPrinter",
+    "//llvm/lib/CodeGen/SelectionDAG",
+    "//llvm/lib/IR",
+    "//llvm/lib/MC",
+    "//llvm/lib/Support",
+    "//llvm/lib/Target",
+  ]
+  include_dirs = [ "." ]
+  sources = [
+    "RISCVAsmPrinter.cpp",
+    "RISCVExpandPseudoInsts.cpp",
+    "RISCVFrameLowering.cpp",
+    "RISCVISelDAGToDAG.cpp",
+    "RISCVISelLowering.cpp",
+    "RISCVInstrInfo.cpp",
+    "RISCVMCInstLower.cpp",
+    "RISCVMergeBaseOffset.cpp",
+    "RISCVRegisterInfo.cpp",
+    "RISCVSubtarget.cpp",
+    "RISCVTargetMachine.cpp",
+    "RISCVTargetObjectFile.cpp",
+  ]
+}
+
+# This is a bit different from most build files: Due to this group
+# having the directory's name, "//llvm/lib/Target/RISCV" will refer to this
+# target, which pulls in the code in this directory *and all subdirectories*.
+# For most other directories, "//llvm/lib/Foo" only pulls in the code directly
+# in "llvm/lib/Foo". The forwarding targets in //llvm/lib/Target expect this
+# different behavior.
+group("RISCV") {
+  deps = [
+    ":LLVMRISCVCodeGen",
+    "AsmParser",
+    "Disassembler",
+    "MCTargetDesc",
+    "TargetInfo",
+    "Utils",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/Disassembler/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("RISCVGenDisassemblerTables") {
+  visibility = [ ":Disassembler" ]
+  args = [ "-gen-disassembler" ]
+  td_file = "../RISCV.td"
+}
+
+static_library("Disassembler") {
+  output_name = "LLVMRISCVDisassembler"
+  deps = [
+    ":RISCVGenDisassemblerTables",
+    "//llvm/lib/MC/MCDisassembler",
+    "//llvm/lib/Support",
+    "//llvm/lib/Target/RISCV/MCTargetDesc",
+    "//llvm/lib/Target/RISCV/Utils",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVDisassembler.cpp",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/MCTargetDesc/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("RISCVGenAsmWriter") {
+  visibility = [ ":MCTargetDesc" ]
+  args = [ "-gen-asm-writer" ]
+  td_file = "../RISCV.td"
+}
+
+tablegen("RISCVGenInstrInfo") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-instr-info" ]
+  td_file = "../RISCV.td"
+}
+
+tablegen("RISCVGenMCCodeEmitter") {
+  visibility = [ ":MCTargetDesc" ]
+  args = [ "-gen-emitter" ]
+  td_file = "../RISCV.td"
+}
+
+tablegen("RISCVGenRegisterInfo") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-register-info" ]
+  td_file = "../RISCV.td"
+}
+
+tablegen("RISCVGenSubtargetInfo") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-subtarget" ]
+  td_file = "../RISCV.td"
+}
+
+# This should contain tablegen targets generating .inc files included
+# by other targets. .inc files only used by .cpp files in this directory
+# should be in deps on the static_library instead.
+group("tablegen") {
+  visibility = [
+    ":MCTargetDesc",
+    "../Utils",
+  ]
+  public_deps = [
+    ":RISCVGenInstrInfo",
+    ":RISCVGenRegisterInfo",
+    ":RISCVGenSubtargetInfo",
+  ]
+}
+
+static_library("MCTargetDesc") {
+  output_name = "LLVMRISCVDesc"
+  public_deps = [
+    ":tablegen",
+  ]
+  deps = [
+    ":RISCVGenAsmWriter",
+    ":RISCVGenMCCodeEmitter",
+    "//llvm/lib/MC",
+    "//llvm/lib/Support",
+    "//llvm/lib/Target/RISCV:RISCVGenCompressInstEmitter",
+    "//llvm/lib/Target/RISCV/Utils",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVAsmBackend.cpp",
+    "RISCVELFObjectWriter.cpp",
+    "RISCVELFStreamer.cpp",
+    "RISCVInstPrinter.cpp",
+    "RISCVMCAsmInfo.cpp",
+    "RISCVMCCodeEmitter.cpp",
+    "RISCVMCExpr.cpp",
+    "RISCVMCTargetDesc.cpp",
+    "RISCVTargetStreamer.cpp",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/TargetInfo/BUILD.gn

+static_library("TargetInfo") {
+  output_name = "LLVMRISCVInfo"
+  deps = [
+    "//llvm/lib/Support",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVTargetInfo.cpp",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/RISCV/Utils/BUILD.gn

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("RISCVGenSystemOperands") {
+  visibility = [ ":Utils" ]
+  args = [ "-gen-searchable-tables" ]
+  td_file = "../RISCV.td"
+}
+
+static_library("Utils") {
+  output_name = "LLVMRISCVUtils"
+  public_deps = [
+    ":RISCVGenSystemOperands",
+  ]
+  deps = [
+    "//llvm/lib/MC",
+    "//llvm/lib/Support",
+    "//llvm/lib/Target/RISCV/MCTargetDesc:tablegen",
+  ]
+
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVBaseInfo.cpp",
+    "RISCVMatInt.cpp",
+  ]
+}

llvm/trunk/utils/gn/secondary/llvm/lib/Target/targets.gni

foreach(target, llvm_targets_to_build) {
   } else if (target == "BPF") {
     llvm_build_BPF = true
   } else if (target == "Hexagon") {
     # Nothing to do.
   } else if (target == "Lanai") {
     # Nothing to do.
   } else if (target == "PowerPC") {
     llvm_build_PowerPC = true
+  } else if (target == "RISCV") {
+    # Nothing to do.
   } else if (target == "Sparc") {
     # Nothing to do.
   } else if (target == "WebAssembly") {
     llvm_build_WebAssembly = true
   } else if (target == "X86") {
     llvm_build_X86 = true
   } else {
     # FIXME: Port the remaining targets.