<div dir="ltr">Andreas,<div><br></div><div>I'll be rolling back this change now, as it broke the build because of the circular dependency which Benjamin described below. Circular dependency is bad and we shouldn't make such dependency in LLVM.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 9, 2013 at 2:54 PM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@gmail.com" target="_blank">benny.kra@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
On 09.10.2013, at 21:02, Anders Waldenborg <<a href="mailto:anders@0x63.nu">anders@0x63.nu</a>> wrote:<br>
<br>
> Author: andersg<br>
> Date: Wed Oct 9 14:02:09 2013<br>
> New Revision: 192316<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=192316&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=192316&view=rev</a><br>
> Log:<br>
> llvm-c: Make target initializer functions external functions in lib.<br>
><br>
> Making them proper functions defined in the (shared)lib instead of<br>
> static inlines defined in the header files makes it possible to<br>
> actually distribute a binary compiled against the shared library<br>
> without having to worry about getting undefined symbol errors when<br>
> calling e.g LLVMInitializeAllTargetInfos because the shared library on<br>
> the other system was compiled with different targets.<br>
<br>
</div>Sorry, I missed the code review for this change. But there's a very good reason why this functionality was never part of llvm-c. Your commit adds a big layering violation to LLVM by adding a cyclic dependency.<br>
<br>
The individual LLVMInitializeXXXFunctions are part of the backends, now they're getting linked into libTarget. This creates a dependency from libTarget to all of the backends, which themselves depend on libTarget. That will break various users of LLVM which don't link everything into one big binary.<br>
<br>
I only see one way to fix this, and it's a bit nasty. Create a new sublibrary which depends on libTarget and all of the targets and put the function entry points there. Users can link that sublibrary when they need the functionality and it avoids adding cycles to the dependency graph.<br>
<br>
- Ben<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D1714" target="_blank">http://llvm-reviews.chandlerc.com/D1714</a><br>
><br>
><br>
> Added:<br>
> llvm/trunk/lib/Target/AllTargets.cpp<br>
> Modified:<br>
> llvm/trunk/include/llvm-c/Target.h<br>
> llvm/trunk/lib/Target/CMakeLists.txt<br>
><br>
> Modified: llvm/trunk/include/llvm-c/Target.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Target.h?rev=192316&r1=192315&r2=192316&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Target.h?rev=192316&r1=192315&r2=192316&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/include/llvm-c/Target.h (original)<br>
> +++ llvm/trunk/include/llvm-c/Target.h Wed Oct 9 14:02:09 2013<br>
> @@ -71,62 +71,37 @@ typedef struct LLVMStructLayout *LLVMStr<br>
> void LLVMInitialize##TargetName##Disassembler(void);<br>
> #include "llvm/Config/Disassemblers.def"<br>
> #undef LLVM_DISASSEMBLER /* Explicit undef to make SWIG happier */<br>
> -<br>
> +<br>
> /** LLVMInitializeAllTargetInfos - The main program should call this function if<br>
> it wants access to all available targets that LLVM is configured to<br>
> support. */<br>
> -static inline void LLVMInitializeAllTargetInfos(void) {<br>
> -#define LLVM_TARGET(TargetName) LLVMInitialize##TargetName##TargetInfo();<br>
> -#include "llvm/Config/Targets.def"<br>
> -#undef LLVM_TARGET /* Explicit undef to make SWIG happier */<br>
> -}<br>
> +void LLVMInitializeAllTargetInfos(void);<br>
><br>
> /** LLVMInitializeAllTargets - The main program should call this function if it<br>
> wants to link in all available targets that LLVM is configured to<br>
> support. */<br>
> -static inline void LLVMInitializeAllTargets(void) {<br>
> -#define LLVM_TARGET(TargetName) LLVMInitialize##TargetName##Target();<br>
> -#include "llvm/Config/Targets.def"<br>
> -#undef LLVM_TARGET /* Explicit undef to make SWIG happier */<br>
> -}<br>
> +void LLVMInitializeAllTargets(void);<br>
><br>
> /** LLVMInitializeAllTargetMCs - The main program should call this function if<br>
> it wants access to all available target MC that LLVM is configured to<br>
> support. */<br>
> -static inline void LLVMInitializeAllTargetMCs(void) {<br>
> -#define LLVM_TARGET(TargetName) LLVMInitialize##TargetName##TargetMC();<br>
> -#include "llvm/Config/Targets.def"<br>
> -#undef LLVM_TARGET /* Explicit undef to make SWIG happier */<br>
> -}<br>
> -<br>
> +void LLVMInitializeAllTargetMCs(void);<br>
> +<br>
> /** LLVMInitializeAllAsmPrinters - The main program should call this function if<br>
> it wants all asm printers that LLVM is configured to support, to make them<br>
> available via the TargetRegistry. */<br>
> -static inline void LLVMInitializeAllAsmPrinters(void) {<br>
> -#define LLVM_ASM_PRINTER(TargetName) LLVMInitialize##TargetName##AsmPrinter();<br>
> -#include "llvm/Config/AsmPrinters.def"<br>
> -#undef LLVM_ASM_PRINTER /* Explicit undef to make SWIG happier */<br>
> -}<br>
> -<br>
> +void LLVMInitializeAllAsmPrinters(void);<br>
> +<br>
> /** LLVMInitializeAllAsmParsers - The main program should call this function if<br>
> it wants all asm parsers that LLVM is configured to support, to make them<br>
> available via the TargetRegistry. */<br>
> -static inline void LLVMInitializeAllAsmParsers(void) {<br>
> -#define LLVM_ASM_PARSER(TargetName) LLVMInitialize##TargetName##AsmParser();<br>
> -#include "llvm/Config/AsmParsers.def"<br>
> -#undef LLVM_ASM_PARSER /* Explicit undef to make SWIG happier */<br>
> -}<br>
> -<br>
> +void LLVMInitializeAllAsmParsers(void);<br>
> +<br>
> /** LLVMInitializeAllDisassemblers - The main program should call this function<br>
> if it wants all disassemblers that LLVM is configured to support, to make<br>
> them available via the TargetRegistry. */<br>
> -static inline void LLVMInitializeAllDisassemblers(void) {<br>
> -#define LLVM_DISASSEMBLER(TargetName) \<br>
> - LLVMInitialize##TargetName##Disassembler();<br>
> -#include "llvm/Config/Disassemblers.def"<br>
> -#undef LLVM_DISASSEMBLER /* Explicit undef to make SWIG happier */<br>
> -}<br>
> -<br>
> +void LLVMInitializeAllDisassemblers(void);<br>
> +<br>
> /** LLVMInitializeNativeTarget - The main program should call this function to<br>
> initialize the native target corresponding to the host. This is useful<br>
> for JIT applications to ensure that the target gets linked in correctly. */<br>
><br>
> Added: llvm/trunk/lib/Target/AllTargets.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AllTargets.cpp?rev=192316&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AllTargets.cpp?rev=192316&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/AllTargets.cpp (added)<br>
> +++ llvm/trunk/lib/Target/AllTargets.cpp Wed Oct 9 14:02:09 2013<br>
> @@ -0,0 +1,43 @@<br>
> +//===-- AllTargets.cpp ----------------------------------------------------===//<br>
> +//<br>
> +// The LLVM Compiler Infrastructure<br>
> +//<br>
> +// This file is distributed under the University of Illinois Open Source<br>
> +// License. See LICENSE.TXT for details.<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +//<br>
> +// This file implements functions for initialization of different<br>
> +// aspects of all configured targets. When calling any of these<br>
> +// functions all configured targets must be linked in.<br>
> +//<br>
> +//===----------------------------------------------------------------------===//<br>
> +<br>
> +#include "llvm-c/Target.h"<br>
> +#include "llvm/Support/TargetSelect.h"<br>
> +<br>
> +using namespace llvm;<br>
> +<br>
> +void LLVMInitializeAllTargetInfos(void) {<br>
> + InitializeAllTargetInfos();<br>
> +}<br>
> +<br>
> +void LLVMInitializeAllTargets(void) {<br>
> + InitializeAllTargets();<br>
> +}<br>
> +<br>
> +void LLVMInitializeAllTargetMCs(void) {<br>
> + InitializeAllTargetMCs();<br>
> +}<br>
> +<br>
> +void LLVMInitializeAllAsmPrinters(void) {<br>
> + InitializeAllAsmPrinters();<br>
> +}<br>
> +<br>
> +void LLVMInitializeAllAsmParsers(void) {<br>
> + InitializeAllAsmParsers();<br>
> +}<br>
> +<br>
> +void LLVMInitializeAllDisassemblers(void) {<br>
> + InitializeAllDisassemblers();<br>
> +}<br>
><br>
> Modified: llvm/trunk/lib/Target/CMakeLists.txt<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/CMakeLists.txt?rev=192316&r1=192315&r2=192316&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/CMakeLists.txt?rev=192316&r1=192315&r2=192316&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Target/CMakeLists.txt (original)<br>
> +++ llvm/trunk/lib/Target/CMakeLists.txt Wed Oct 9 14:02:09 2013<br>
> @@ -1,4 +1,5 @@<br>
> add_llvm_library(LLVMTarget<br>
> + AllTargets.cpp<br>
> Mangler.cpp<br>
> Target.cpp<br>
> TargetIntrinsicInfo.cpp<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>