[GlobalISel] Add the proper cmake plumbing.

[qcolombet]

Summary

Hi,

This patch adds the necessary plumbing to cmake to build the sources related to GlobalISel.
To be able to test this plumbing, this patch also adds an empty implementation of the IRTranslator.
Do not waste time on this empty implementation, the API will likely evolve as the prototype goes.

After this patch lands, it will be possible to build sources related to GlobalISel by adding -DBUILD_GLOBAL_ISEL=ON. By default, this is OFF, thus GlobalISel sources will not impact people that do not explicitly opt-in.

OK to commit?

Thanks,
-Quentin

[jmolloy]

Hi Quentin,

There should be a line in the top-level CMakeLists.txt exposing BUILD_GLOBAL_ISEL to the cache. Otherwise, if you re-cmake, you won't build global isel the second time!

Something like

set(BUILD_GLOBAL_ISEL 0 CACHE STRING "Experimental: Build GlobalISel")

I'd also prefer this to have a "LLVM_" prefix as all other global cache variables do in our CMakeLists.

Cheers,

James

[qcolombet]

• Add LLVM prefix to the variable name
• Set the variable into the cache

[qcolombet]

Hi James,

Thanks for the feedbacks.
I’ve updated the patch accordingly.

Cheers,
-Quentin

[MatzeB]

The commit reads somewhat strange as it looks like a TODO file split into comments. I guess that is fine and too early to review until we have some real implementations. The cmake parts look good to me now. Nitpicks below.

include/llvm/CodeGen/GlobalISel/IRTranslator.h

25 ↗

(On Diff #44336)

Isn't this obvious?

31–38 ↗

(On Diff #44336)

Before the class I would expect a doxygen (///) comment describing what the pass does.
I think high-level concerns and discussions should rather go into the \file section of the .cpp file.

The same applies to most of the following comments, they are not doxygen and often describe a lot of high-level concerns instead of just describing what the function does...

44 ↗

(On Diff #44336)

grammar.

65 ↗

(On Diff #44336)

grammar. Could use doxygen style // .. // when describing a larger section of the file.

lib/CodeGen/GlobalISel/LLVMBuild.txt

18–22 ↗

(On Diff #44336)

Does this give us a way to disable/enable GlobalISel in the Makefiles? If not maybe just leave out of the Makefile system for now as there is a good chance that the Makefiles are removed before GlobalISel is relevant for most people.

[qcolombet]

Hi Matthias,

The commit reads somewhat strange as it looks like a TODO file split into comments.

This is really what this is :). I could it I guess to have just the cmake part, but we wouldn’t have anything to test the cmake then.

I didn’t quite get your comment on the "Makefile part”. I am admittedly not familiar with the internals of the build system. See my inlined comments.

Thanks,
-Quentin

include/llvm/CodeGen/GlobalISel/IRTranslator.h

25 ↗

(On Diff #44336)

I don’t know how common it is to use that kind of comments, but I personally like to call it out so that it is obvious that this section is for forward declarations and nothing else.
I can take it out if you prefer.

31–38 ↗

(On Diff #44336)

Yeah, at this point, it is more todos than actual doxygen comments.

lib/CodeGen/GlobalISel/LLVMBuild.txt

18–22 ↗

(On Diff #44336)

What do you mean “in the Makefiles”?

If you do add the dependency on GlobalISel, the libLLVMGlobalISel should be built, right?

Moreover, I thought those LLVMBuild.txt files were mandatory.

[MatzeB]

include/llvm/CodeGen/GlobalISel/IRTranslator.h

25 ↗

(On Diff #44336)

It's probably a personal thing but I often think a reduced amount of information is better than large amounts of obvious/unnecessary information as there is always a (really really tiny in this case) cost of reading/processing it. Sure this particular example is probably not even worth the time thinking or discussing it in a review. So no objection from me one way or the other.

lib/CodeGen/GlobalISel/LLVMBuild.txt

18–22 ↗

(On Diff #44336)

Hmm I assumed the LLVMBuild.txt is only used for the Makefile build, because when I added some simple libraries/executables in my personal experiments everything worked fine without them and the information looked like a duplication of everything in CMakeLists.txt. But you are right I just checked again and the cmake build is doing something with the information as well, so they are apparently necessary for cmake and Makefile builds.

[qcolombet]

• Grammar.
• Add a comment stating that the comments need proper doxygen style.

[qcolombet]

Typo in comment.

[qcolombet]

Hi Matthias,

Is it okay to commit?

Cheers,
-Quentin

[MatzeB]

In D15983#330230, @qcolombet wrote:

Hi Matthias,

Is it okay to commit?

sure.

[echristo]

I'm going to giggle a bit at the use of Toolkit, but LGTM as well.

Thanks!

-eric

[ovyalov]

It seems lldb xcode build started to fail after this CL - http://lab.llvm.org:8011/builders/lldb-x86_64-darwin-13.4/builds/8218

llvm-config: error: missing: /Users/lldb_build/lldbSlave/buildDir/lldb/llvm-build/Release+Asserts/x86_64/Release+Asserts/lib/libLLVMGlobalISel.a
/Users/lldb_build/lldbSlave/buildDir/lldb/llvm/Makefile.rules:1080: *** llvm-config --libs failed. Stop.

Can you provide Makefile configuration for this library?

[llvm-commits]

I believe r258379 should fix the problem.

Let me know if this is not the case.

Thanks,
-Quentin

llvm/trunk/CMakeLists.txt

if(LLVM_PARALLEL_COMPILE_JOBS)
   if(CMAKE_VERSION VERSION_LESS 3.0 OR NOT CMAKE_MAKE_PROGRAM MATCHES "ninja")
     message(WARNING "Job pooling is only available with Ninja generators and CMake 3.0 and later.")
   else()
     set_property(GLOBAL APPEND PROPERTY JOB_POOLS compile_job_pool=${LLVM_PARALLEL_COMPILE_JOBS})
     set(CMAKE_JOB_POOL_COMPILE compile_job_pool)
   endif()
 endif()
 
+set(LLVM_BUILD_GLOBAL_ISEL OFF CACHE BOOL "Experimental: Build GlobalISel")
+
 set(LLVM_PARALLEL_LINK_JOBS "" CACHE STRING
   "Define the maximum number of concurrent link jobs.")
 if(LLVM_PARALLEL_LINK_JOBS)
   if(CMAKE_VERSION VERSION_LESS 3.0 OR NOT CMAKE_MAKE_PROGRAM MATCHES "ninja")
     message(WARNING "Job pooling is only available with Ninja generators and CMake 3.0 and later.")
   else()
     set_property(GLOBAL APPEND PROPERTY JOB_POOLS link_job_pool=${LLVM_PARALLEL_LINK_JOBS})
     set(CMAKE_JOB_POOL_LINK link_job_pool)

llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h

+//===-- llvm/CodeGen/GlobalISel/IRTranslator.h - IRTranslator ---*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file declares the IRTranslator pass.
+/// This pass is responsible for translating LLVM IR into MachineInstr.
+/// It uses target hooks to lower the ABI but aside from that, the pass
+/// generated code is generic. This is the default translator used for
+/// GlobalISel.
+///
+/// \todo Replace the comments with actual doxygen comments.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_GLOBALISEL_IRTRANSLATOR_H
+#define LLVM_CODEGEN_GLOBALISEL_IRTRANSLATOR_H
+
+#include "Types.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/IR/Constants.h"
+
+namespace llvm {
+// Forward declarations.
+class Constant;
+class Instruction;
+class MachineInstr;
+class MachineIRBuilder;
+
+// Technically the pass should run on an hypothetical MachineModule,
+// since it should translate Global into some sort of MachineGlobal.
+// The MachineGlobal should ultimately just be a transfer of ownership of
+// the interesting bits that are relevant to represent a global value.
+// That being said, we could investigate what would it cost to just duplicate
+// the information from the LLVM IR.
+// The idea is that ultimately we would be able to free up the memory used
+// by the LLVM IR as soon as the translation is over.
+class IRTranslator : public MachineFunctionPass {
+public:
+  static char ID;
+
+private:
+  // Interface used to lower the everything related to calls.
+  //  TargetLowering *CallLowering;
+  // Mapping of the values of the current LLVM IR function
+  // to the related virtual registers.
+  // We need several virtual registers for the lowering of things
+  // like structures. Right now, this is just a list of virtual
+  // registers, but we would need to encapsulate that in a higher
+  // level class.
+  ValueToVRegs ValToVRegs;
+  // Mapping of a constant to the instructions to produce
+  // that constant.
+  // Constants are special because when we encounter one,
+  // we do not know at first where to insert the definition since
+  // this depends on all its uses.
+  // Thus, we will insert the sequences to materialize them when
+  // we know all their users.
+  // In the meantime, just keep it in a map.
+  // Note: Constants that end up as immediate in the related instructions,
+  // do not appear in that map.
+  DenseMap<const Constant *, SmallVector<MachineInstr *, 1>> ConstantToSequence;
+
+  /* A bunch of methods targeting ADD, SUB, etc. */
+  // Return true if the translation was successful, false
+  // otherwise.
+  // Note: The MachineIRBuilder would encapsulate a
+  // MachineRegisterInfo to create virtual registers.
+  //
+  // Algo:
+  // 1. Look for a virtual register for each operand or
+  //    create one.
+  // 2 Update the ValToVReg accordingly.
+  // 2.alt. For constant arguments, if they are compile time constants,
+  //   produce an immediate in the right operand and do not touch
+  //   ValToReg. Otherwise, update ValToVReg and register the
+  //   sequence to materialize the constant in ConstantToSequence.
+  // 3. Create the generic instruction.
+  bool translateADD(const Instruction &Inst);
+
+  // Builder for machine instruction a la IRBuilder.
+  // I.e., compared to regular MIBuilder, this one also inserts the instruction
+  // in the current block, it can creates block, etc., basically a kind of
+  // IRBuilder, but for Machine IR.
+  MachineIRBuilder *MIRBuilder;
+
+  // Return true if the translation from LLVM IR to Machine IR
+  // suceeded.
+  // See translateXXX for details.
+  bool translate(const Instruction &);
+
+  // * Insert all the code needed to materialize the constants
+  // at the proper place. E.g., Entry block or dominator block
+  // of each constant depending ob how fancy we want to be.
+  // * Clear the different maps.
+  void finalize();
+public:
+  // Ctor, nothing fancy.
+  IRTranslator();
+
+  // Instead of having the instance of the IRTranslatorToolkit
+  // as an argument of the constructor of IRTranslator, we ask
+  // the target the instance of the toolkit for each MachineFunction.
+  // The interest is that we may have different translator for different
+  // subtract or optimization. E.g., we could have a translator optimized
+  // to produce small code size.
+  //
+  // Algo:
+  //   CallLowering = MF.subtarget.getCallLowering()
+  //   F = MF.getParent()
+  //   MIRBuilder.reset(MF)
+  //   MIRBuilder.getOrCreateBB(F.getEntryBB())
+  //   CallLowering->translateArguments(MIRBuilder, F, ValToVReg)
+  //   for each bb in F
+  //     MIRBuilder.getOrCreateBB(bb)
+  //     for each inst in bb
+  //       if (!translate(MIRBuilder, inst, ValToVReg, ConstantToSequence))
+  //         report_fatal_error(“Don’t know how to translate input");
+  //   finalize()
+  bool runOnMachineFunction(MachineFunction &MF) override;
+};
+
+} // End namespace llvm.
+#endif

llvm/trunk/include/llvm/CodeGen/GlobalISel/Types.h

+//===-- llvm/CodeGen/GlobalISel/Types.h - Types used by GISel ----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file describes high level types that are used by several passes or
+/// APIs involved in the GlobalISel pipeline.
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CODEGEN_GLOBALISEL_TYPES_H
+#define LLVM_CODEGEN_GLOBALISEL_TYPES_H
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/Value.h"
+
+namespace llvm {
+
+/// Map a value to virtual registers.
+/// We must support several virtual registers for a value.
+/// Indeed each virtual register is mapped to one EVT, but a value
+/// may span over several EVT when it is a type representing a structure.
+/// In that case the value will be break into EVTs.
+/// Note: We need to expose this type to the target hooks for thing like
+/// ABI lowering that would be used during IRTranslation.
+typedef DenseMap<const Value *, SmallVector<unsigned, 1>> ValueToVRegs;
+
+} // End namespace llvm.
+#endif

llvm/trunk/lib/CodeGen/CMakeLists.txt

add_llvm_library(LLVMCodeGen
   LINK_LIBS ${system_libs}
   )
 
 add_dependencies(LLVMCodeGen intrinsics_gen)
 
 add_subdirectory(SelectionDAG)
 add_subdirectory(AsmPrinter)
 add_subdirectory(MIRParser)
+if(LLVM_BUILD_GLOBAL_ISEL)
+  add_subdirectory(GlobalISel)
+endif()

llvm/trunk/lib/CodeGen/GlobalISel/CMakeLists.txt

+add_llvm_library(LLVMGlobalISel
+        IRTranslator.cpp
+  )
+
+add_dependencies(LLVMGlobalISel intrinsics_gen)

llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp

+//===-- llvm/CodeGen/GlobalISel/IRTranslator.cpp - IRTranslator --*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// This file implements the IRTranslator class.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/CodeGen/GlobalISel/IRTranslator.h"
+
+using namespace llvm;
+
+char IRTranslator::ID = 0;
+
+bool IRTranslator::translateADD(const Instruction &Inst) {
+  return false;
+}
+
+bool IRTranslator::translate(const Instruction &) {
+  return false;
+}
+
+
+void IRTranslator::finalize() {
+}
+
+IRTranslator::IRTranslator()
+  : MachineFunctionPass(ID) {
+}
+
+bool IRTranslator::runOnMachineFunction(MachineFunction &MF) {
+  return false;
+}

llvm/trunk/lib/CodeGen/GlobalISel/LLVMBuild.txt

+;===- ./lib/CodeGen/GlobalISel/LLVMBuild.txt -----------------*- Conf -*--===;
+;
+;                     The LLVM Compiler Infrastructure
+;
+; This file is distributed under the University of Illinois Open Source
+; License. See LICENSE.TXT for details.
+;
+;===------------------------------------------------------------------------===;
+;
+; This is an LLVMBuild description file for the components in this subdirectory.
+;
+; For more information on the LLVMBuild system, please see:
+;
+;   http://llvm.org/docs/LLVMBuild.html
+;
+;===------------------------------------------------------------------------===;
+
+[component_0]
+type = Library
+name = GlobalISel
+parent = CodeGen
+required_libraries = Analysis CodeGen Core MC Support Target TransformUtils

llvm/trunk/lib/CodeGen/LLVMBuild.txt

 ;
 ; For more information on the LLVMBuild system, please see:
 ;
 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [common]
-subdirectories = AsmPrinter SelectionDAG MIRParser
+subdirectories = AsmPrinter SelectionDAG MIRParser GlobalISel
 
 [component_0]
 type = Library
 name = CodeGen
 parent = Libraries
 required_libraries = Analysis BitReader BitWriter Core Instrumentation MC Scalar Support Target TransformUtils

llvm/trunk/tools/llc/CMakeLists.txt

+# Add GlobalISel to the dependencies if the user wants to build it.
+if(LLVM_BUILD_GLOBAL_ISEL)
+  set(GLOBAL_ISEL GlobalISel)
+else()
+  set(GLOBAL_ISEL "")
+endif()
+
 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Analysis
   AsmPrinter
   CodeGen
   Core
   IRReader
+  ${GLOBAL_ISEL}
   MC
   MIRParser
   ScalarOpts
   SelectionDAG
   Support
   Target
   TransformUtils
   )
 
 # Support plugins.
 set(LLVM_NO_DEAD_STRIP 1)
 
 add_llvm_tool(llc
   llc.cpp
   )
 export_executable_symbols(llc)