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

31

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

5

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

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

12

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

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

44

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

31

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

Oops, sorry about that...

[thakis]

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

31

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

31

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/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:tablegen",
+    "//llvm/lib/Target/RISCV/MCTargetDesc",
+    "//llvm/lib/Target/RISCV/Utils",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVAsmParser.cpp",
+  ]
+}

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

+import("//llvm/utils/TableGen/tablegen.gni")
+
+tablegen("RISCVGenCompressInstEmitter") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-compress-inst-emitter" ]
+  td_file = "RISCV.td"
+}
+
+tablegen("RISCVGenDAGISel") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-dag-isel" ]
+  td_file = "RISCV.td"
+}
+
+tablegen("RISCVGenMCPseudoLowering") {
+  visibility = [ ":tablegen" ]
+  args = [ "-gen-pseudo-lowering" ]
+  td_file = "RISCV.td"
+}
+
+group("tablegen") {
+  visibility = [
+    ":LLVMRISCVCodeGen",
+    "AsmParser",
+    "MCTargetDesc",
+    "Utils",
+  ]
+  public_deps = [
+    ":RISCVGenCompressInstEmitter",
+    ":RISCVGenDAGISel",
+    ":RISCVGenMCPseudoLowering",
+  ]
+}
+
+static_library("LLVMRISCVCodeGen") {
+  deps = [
+    ":tablegen",
+    "MCTargetDesc",
+    "TargetInfo",
+    "//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",
+    "RISCVInstrInfo.cpp",
+    "RISCVISelDAGToDAG.cpp",
+    "RISCVISelLowering.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/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/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:tablegen",
+    "//llvm/lib/Target/RISCV/Utils",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+    "RISCVAsmBackend.cpp",
+    "RISCVELFObjectWriter.cpp",
+    "RISCVInstPrinter.cpp",
+    "RISCVMCAsmInfo.cpp",
+    "RISCVMCCodeEmitter.cpp",
+    "RISCVMCExpr.cpp",
+    "RISCVMCTargetDesc.cpp",
+    "RISCVTargetStreamer.cpp",
+    "RISCVELFStreamer.cpp",
+  ]
+}

llvm/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/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/utils/gn/secondary/llvm/lib/Target/targets.gni

if (host_cpu == "arm64") {
     assert(false, "add your host_cpu above")
   }
 } else if (llvm_targets_to_build == "all") {
   # FIXME: Port the remaining targets.
   llvm_targets_to_build = [
     "AArch64",
     "ARM",
     "BPF",
     "Hexagon",
     "Lanai",
-    "PowerPC",
+    "PowerPC",
+    "RISCV",
     "Sparc",
     "WebAssembly",
     "X86",
   ]
 }
 
 # Validate that llvm_targets_to_build is set to a list of valid targets,
 # and remember which targets are built.
 llvm_build_AArch64 = false
 llvm_build_ARM = false
 llvm_build_BPF = false
 llvm_build_PowerPC = false
 llvm_build_WebAssembly = false
 llvm_build_X86 = false
 foreach(target, llvm_targets_to_build) {
   if (target == "AArch64") {
     llvm_build_AArch64 = true
   } else if (target == "ARM") {
     llvm_build_ARM = true
   } 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 == "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.
     assert(false, "Unknown target '$target'.")
   }
 }
 
 # FIXME: This should be based off target_cpu once cross compiles work.
 if (host_cpu == "arm64") {
   native_target = "AArch64"
 } else if (host_cpu == "arm") {
   native_target = "ARM"
 } else if (host_cpu == "ppc" || host_cpu == "ppc64") {
   native_target = "PowerPC"
 } else if (host_cpu == "x86" || host_cpu == "x64") {
   native_target = "X86"
 } else {
 }