[llvm-commits] [llvm] r75739 - in /llvm/trunk: include/llvm/Target/TargetRegistry.h lib/Support/TargetRegistry.cpp

Daniel Dunbar daniel at zuster.org
Wed Jul 15 20:32:18 PDT 2009


How about this design for the registration mechanisms. Instead of
worrying about Target, it encapsulates the registration mechanisms
inside helper templates, so we can leave Target being plain and simple
and guaranteed-constructor-free.

--
Index: include/llvm/Target/TargetRegistry.h
===================================================================
--- include/llvm/Target/TargetRegistry.h	(revision 75891)
+++ include/llvm/Target/TargetRegistry.h	(working copy)
@@ -19,6 +19,7 @@
 #ifndef LLVM_TARGET_TARGETREGISTRY_H
 #define LLVM_TARGET_TARGETREGISTRY_H

+#include "llvm/Target/TargetMachine.h"
 #include <string>
 #include <cassert>

@@ -246,6 +247,79 @@
     /// @}
   };

+
+  //===--------------------------------------------------------------------===//
+
+  /// RegisterTarget - Helper template for registering a target, for use in the
+  /// target's initialization function. Usage:
+  ///
+  ///
+  /// Target TheFooTarget; // The global target instance.
+  ///
+  /// namespace {
+  ///   struct FooInfo {
+  ///     static unsigned getJITMatchQuality() { ... }
+  ///     static unsigned getTripleMatchQuality(const std::string &) { ... }
+  ///     static unsigned getModuleMatchQuality(const Module &) { ... }
+  ///   };
+  /// }
+  ///
+  /// extern "C" void LLVMInitializeFooTargetInfo() {
+  ///   RegisterTarget<FooAsmPrinter> X(TheFooTarget, "foo", "Foo
description");
+  /// }
+  template<class TargetInfoImpl>
+  struct RegisterTarget {
+    RegisterTarget(Target &T, const char *Name, const char *Desc) {
+      TargetRegistry::RegisterTarget(T, Name, Desc,
+                                     &TargetInfoImpl::getTripleMatchQuality,
+                                     &TargetInfoImpl::getModuleMatchQuality,
+                                     &TargetInfoImpl::getJITMatchQuality);
+    }
+  };
+
+  /// RegisterTargetMachine - Helper template for registering a target machine
+  /// implementation, for use in the target machine initialization
+  /// function. Usage:
+  ///
+  /// extern "C" void LLVMInitializeFooTarget() {
+  ///   extern Target TheFooTarget;
+  ///   RegisterTargetMachine<FooTargetMachine> X(TheFooTarget);
+  /// }
+  template<class TargetMachineImpl>
+  struct RegisterTargetMachine {
+    RegisterTargetMachine(Target &T) {
+      TargetRegistry::RegisterTargetMachine(T, &Allocator);
+    }
+
+  private:
+    static TargetMachine *Allocator(const Target &T, const Module &M,
+                                    const std::string &FS) {
+      return new TargetMachineImpl(T, M, FS);
+    }
+  };
+
+  /// RegisterAsmPrinter - Helper template for registering a target machine
+  /// implementation, for use in the target machine initialization
+  /// function. Usage:
+  ///
+  /// extern "C" void LLVMInitializeFooAsmPrinter() {
+  ///   extern Target TheFooTarget;
+  ///   RegisterAsmPrinter<FooAsmPrinter> X(TheFooTarget);
+  /// }
+  template<class AsmPrinterImpl>
+  struct RegisterAsmPrinter {
+    RegisterAsmPrinter(Target &T) {
+      TargetRegistry::RegisterAsmPrinter(T, &Allocator);
+    }
+
+  private:
+    static FunctionPass *Allocator(formatted_raw_ostream &OS,
+                                   TargetMachine &TM,
+                                   bool Verbose) {
+      return new AsmPrinterImpl(OS, TM, TM.getTargetAsmInfo(), Verbose);
+    }
+  };
+
 }

 #endif
Index: lib/Target/X86/X86TargetMachine.cpp
===================================================================
--- lib/Target/X86/X86TargetMachine.cpp	(revision 75886)
+++ lib/Target/X86/X86TargetMachine.cpp	(working copy)
@@ -20,21 +20,17 @@
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Target/TargetOptions.h"
-#include "llvm/Target/TargetMachineRegistry.h"
+#include "llvm/Target/TargetRegistry.h"
 using namespace llvm;

-// Register the target.
-extern Target TheX86_32Target;
-static RegisterTarget<X86_32TargetMachine>
-X(TheX86_32Target, "x86",    "32-bit X86: Pentium-Pro and above");
-
-extern Target TheX86_64Target;
-static RegisterTarget<X86_64TargetMachine>
-Y(TheX86_64Target, "x86-64", "64-bit X86: EM64T and AMD64");
-
 // Force static initialization.
 extern "C" void LLVMInitializeX86Target() {
-
+  // Register the target machines.
+  extern Target TheX86_32Target;
+  RegisterTargetMachine<X86_32TargetMachine> X(TheX86_32Target);
+
+  extern Target TheX86_64Target;
+  RegisterTargetMachine<X86_64TargetMachine> Y(TheX86_64Target);
 }

 const TargetAsmInfo *X86TargetMachine::createTargetAsmInfo() const {
Index: lib/Target/X86/TargetInfo/X86TargetInfo.cpp
===================================================================
--- lib/Target/X86/TargetInfo/X86TargetInfo.cpp	(revision 75886)
+++ lib/Target/X86/TargetInfo/X86TargetInfo.cpp	(working copy)
@@ -11,88 +11,90 @@
 #include "llvm/Target/TargetRegistry.h"
 using namespace llvm;

-Target TheX86_32Target;
-
-static unsigned X86_32_JITMatchQuality() {
+namespace {
+  struct X86_32Info {
+    static unsigned getJITMatchQuality() {
 #if defined(i386) || defined(__i386__) || defined(__x86__) || defined(_M_IX86)
-  return 10;
+      return 10;
 #endif
-  return 0;
-}
+      return 0;
+    }

-static unsigned X86_32_TripleMatchQuality(const std::string &TT) {
-  // We strongly match "i[3-9]86-*".
-  if (TT.size() >= 5 && TT[0] == 'i' && TT[2] == '8' && TT[3] == '6' &&
-      TT[4] == '-' && TT[1] - '3' < 6)
-    return 20;
+    static unsigned getTripleMatchQuality(const std::string &TT) {
+      // We strongly match "i[3-9]86-*".
+      if (TT.size() >= 5 && TT[0] == 'i' && TT[2] == '8' && TT[3] == '6' &&
+          TT[4] == '-' && TT[1] - '3' < 6)
+        return 20;
+
+      return 0;
+    }
+
+    static unsigned getModuleMatchQuality(const Module &M) {
+      // Check for a triple match.
+      if (unsigned Q = getTripleMatchQuality(M.getTargetTriple()))
+        return Q;
+
+      // If the target triple is something non-X86, we don't match.
+      if (!M.getTargetTriple().empty()) return 0;
+
+      if (M.getEndianness()  == Module::LittleEndian &&
+          M.getPointerSize() == Module::Pointer32)
+        return 10;                               // Weak match
+      else if (M.getEndianness() != Module::AnyEndianness ||
+               M.getPointerSize() != Module::AnyPointerSize)
+        return 0;                                // Match for some other target

-  return 0;
+      return getJITMatchQuality()/2;
+    }
+  };
 }

-static unsigned X86_32_ModuleMatchQuality(const Module &M) {
-  // Check for a triple match.
-  if (unsigned Q = X86_32_TripleMatchQuality(M.getTargetTriple()))
-    return Q;
-
-  // If the target triple is something non-X86, we don't match.
-  if (!M.getTargetTriple().empty()) return 0;
-
-  if (M.getEndianness()  == Module::LittleEndian &&
-      M.getPointerSize() == Module::Pointer32)
-    return 10;                                   // Weak match
-  else if (M.getEndianness() != Module::AnyEndianness ||
-           M.getPointerSize() != Module::AnyPointerSize)
-    return 0;                                    // Match for some other target
-
-  return X86_32_JITMatchQuality()/2;
-}
-
-Target TheX86_64Target;
-
-static unsigned X86_64_JITMatchQuality() {
+namespace {
+  struct X86_64Info {
+    static unsigned getJITMatchQuality() {
 #if defined(__x86_64__) || defined(_M_AMD64)
-  return 10;
+      return 10;
 #endif
-  return 0;
-}
+      return 0;
+    }

-static unsigned X86_64_TripleMatchQuality(const std::string &TT) {
-  // We strongly match "x86_64-*".
-  if (TT.size() >= 7 && TT[0] == 'x' && TT[1] == '8' && TT[2] == '6' &&
-      TT[3] == '_' && TT[4] == '6' && TT[5] == '4' && TT[6] == '-')
-    return 20;
-
-  return 0;
+    static unsigned getTripleMatchQuality(const std::string &TT) {
+      // We strongly match "x86_64-*".
+      if (TT.size() >= 7 && TT[0] == 'x' && TT[1] == '8' && TT[2] == '6' &&
+          TT[3] == '_' && TT[4] == '6' && TT[5] == '4' && TT[6] == '-')
+        return 20;
+
+      return 0;
+    }
+
+    static unsigned getModuleMatchQuality(const Module &M) {
+      // Check for a triple match.
+      if (unsigned Q = getTripleMatchQuality(M.getTargetTriple()))
+        return Q;
+
+      // If the target triple is something non-X86-64, we don't match.
+      if (!M.getTargetTriple().empty()) return 0;
+
+      if (M.getEndianness()  == Module::LittleEndian &&
+          M.getPointerSize() == Module::Pointer64)
+        return 10;                               // Weak match
+      else if (M.getEndianness() != Module::AnyEndianness ||
+               M.getPointerSize() != Module::AnyPointerSize)
+        return 0;                                // Match for some other target
+
+      return getJITMatchQuality()/2;
+    }
+  };
 }

-static unsigned X86_64_ModuleMatchQuality(const Module &M) {
-  // Check for a triple match.
-  if (unsigned Q = X86_64_TripleMatchQuality(M.getTargetTriple()))
-    return Q;
+Target TheX86_32Target;

-  // If the target triple is something non-X86-64, we don't match.
-  if (!M.getTargetTriple().empty()) return 0;
+Target TheX86_64Target;

-  if (M.getEndianness()  == Module::LittleEndian &&
-      M.getPointerSize() == Module::Pointer64)
-    return 10;                                   // Weak match
-  else if (M.getEndianness() != Module::AnyEndianness ||
-           M.getPointerSize() != Module::AnyPointerSize)
-    return 0;                                    // Match for some other target
-
-  return X86_64_JITMatchQuality()/2;
-}
-
 extern "C" void LLVMInitializeX86TargetInfo() {
-  TargetRegistry::RegisterTarget(TheX86_32Target, "x86",
-                                  "32-bit X86: Pentium-Pro and above",
-                                  &X86_32_TripleMatchQuality,
-                                  &X86_32_ModuleMatchQuality,
-                                  &X86_32_JITMatchQuality);
-
-  TargetRegistry::RegisterTarget(TheX86_64Target, "x86-64",
-                                  "64-bit X86: EM64T and AMD64",
-                                  &X86_64_TripleMatchQuality,
-                                  &X86_64_ModuleMatchQuality,
-                                  &X86_64_JITMatchQuality);
+  RegisterTarget<X86_32Info> X(TheX86_32Target, "x86",
+                               "32-bit X86: Pentium-Pro and above");
+
+  RegisterTarget<X86_64Info> Y(TheX86_64Target, "x86-64",
+                               "64-bit X86: EM64T and AMD64");
 }
--

Not all targets will be able to use the RegisterAsmPrinter template,
since they sometimes choose the implementation based on the
TargetMachine, but many will.

A few responses:

On Tue, Jul 14, 2009 at 11:07 PM, Chris Lattner<clattner at apple.com> wrote:
> On Jul 14, 2009, at 9:25 PM, Daniel Dunbar wrote:
>> +  /// Target - Wrapper for Target specific information.
>> +  ///
>> +  /// For registration purposes, this is a POD type so that targets
>> can be
>> +  /// registered without the use of static constructors.
>
> Please comment that instances of this should be declared as global
> variables, and that this depends on them being zero initialized.

Done. When the dust is settled I will update the 2.6 docs and the
writing a pass guide.

>> +  struct Target {
>
> How about something like this:

See above.

>> +    /// ModuleMatchQualityFn - The target function for rating the
>> match quality
>> +    /// of a module.
>> +    ModuleMatchQualityFnTy ModuleMatchQualityFn;
>
> Instead of returning a random unsigned, this might be a good time to
> introduce an enum for "strong match", "weak match" etc.
> Alternatively, we could just go for "strong match" or "no match".
> There is code that tries to do a fuzzy match based on the target data
> if a triple is not around, this is old and baroque and probably should
> be ripped out.

I agree, but its orthogonal. I think we should rewrite everything in
terms of the triple match function, since that is more or less what
all the targets are doing anyway. That also solves the layering
problem you point out.

>> +  /// TargetRegistry - Generic interface to target specific features.
>> +  //
>> +  // FIXME: Provide Target* iterator.
>> +  struct TargetRegistry {
>
> Please make this a class with all public members.  This simplifies
> forward declaration for VC++.

Changed to a class.

>> +    /// getClosestStaticTargetForModule - Given an LLVM module,
>> pick the best
>> +    /// target that is compatible with the module.  If no close
>> target can be
>> +    /// found, this returns null and sets the Error string to a
>> reason.
>> +    static const Target *getClosestStaticTargetForModule(const
>> Module &M,
>> +                                                        std::string
>> &Error);
>
> This function could be tricky.  We don't want TargetRegistry.cpp to
> depend on VMCore.  This can be worked around by making the part that
> touches Module be inline, and call into an out-of-line version that
> takes the guts of module (e.g. the triple from it).  Alternatively, we
> could just eliminate this method.

I vote for the latter.

>> +    /// RegisterTarget - Register the given target.
>> +    ///
>> +    /// @param T - The target being registered.
>> +    /// @param Name - The target name. This should be a static
>> string.
>> +    /// @param ShortDesc - A short target description. This should
>> be a static
>> +    /// string.
>> +    /// @param TQualityFn - The triple match quality computation
>> function for
>> +    /// this target.
>> +    /// @param MQualityFn - The module match quality computation
>> function for
>> +    /// this target.
>> +    /// @param JITMatchQualityFn - The JIT match quality
>> computation function
>> +    /// for this target.
>> +    static void RegisterTarget(Target &T,
>> +                               const char *Name,
>> +                               const char *ShortDesc,
>> +                               Target::TripleMatchQualityFnTy
>> TQualityFn,
>> +                               Target::ModuleMatchQualityFnTy
>> MQualityFn,
>> +                               Target::JITMatchQualityFnTy
>> JITQualityFn);
>
> Using a template like above would remove some of these, which would be
> nice.  It would be nice for the target to just define its entry point
> like:
>
> extern "C" void InitializeX86Target() {
>   TargetRegistry::RegisterTarget(TheX86Target);
> }

Incorporated above, it just puts the template in a different place.


>> +// FIXME: Worry about locking? In general everything should be
>> registered at
>> +// startup.
>
> Just document the registration methods as having to be called before
> llvm_start_multithreading happens.

Yeah, that was my plan. Clarified comments.

>>
>> +const Target *
>> +TargetRegistry::getClosestStaticTargetForModule(const Module &M,
>> +                                                std::string &Error) {
>> +  Target *Best = 0, *EquallyBest = 0;
>> +  unsigned BestQuality = 0;
>> +  // FIXME: Use iterator.
>> +  for (Target *i = FirstTarget; i; i = i->Next) {
>> +    if (unsigned Qual = i->ModuleMatchQualityFn(M)) {
>
> Ah, so this doesn't depend on vmcore, but it forces the TargetInfo
> implementations to depend on vmcore.  That's not nice either :).
>
> Looks like a great step!
>
> -Chris
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>




More information about the llvm-commits mailing list