[llvm-commits] [llvm] r75739 - in /llvm/trunk: include/llvm/Target/TargetRegistry.h lib/Support/TargetRegistry.cpp
Chris Lattner
clattner at apple.com
Wed Jul 15 21:21:50 PDT 2009
On Jul 15, 2009, at 8:32 PM, Daniel Dunbar wrote:
> 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.
Looks great to me!
-Chris
>
> --
> 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
>>
>
> _______________________________________________
> 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