Bug 45420

Created attachment 56357 [details] [review]
Proposed fix

There is only X86 and ARM LLVM Disassemblers, so initialize only native one, if exist.
More generic solution will be possible after something like 
InitializeNativeTargetDisassembler() will be added to LLVM (filled http://llvm.org/bugs/show_bug.cgi?id=11894 will post follow up Mesa patch after LLVM patch inclusion).

Pushed. Thanks.

mesa: b0337b679ad4c2feae59215104cfa60b58a619d5 (master)

$ llvm-config --version
2.7

$ scons
[...]
  Linking build/linux-x86_64-debug/gallium/drivers/llvmpipe/lp_test_blend ...
build/linux-x86_64-debug/gallium/auxiliary/libgallium.a(lp_bld_debug.os): In function `llvm::InitializeAllAsmPrinters()':
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeMBlazeAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeBlackfinAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeSystemZAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeMSP430AsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeXCoreAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializePIC16AsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeCellSPUAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeMipsAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeARMAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeAlphaAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializePowerPCAsmPrinter'
/usr/include/llvm/Config/AsmPrinters.def:27: undefined reference to `LLVMInitializeSparcAsmPrinter'
collect2: ld returned 1 exit status

Created attachment 56422 [details] [review]
Proposed fix for LLVM-2.7

I don't really know how to properly fix this for all targets, because LLVM_NATIVE_ARCH in LLVM-2.7 is ${LLVM_NATIVE_ARCH}Target (X86Target for X86, ARMTarget for ARM etc), so it's not possible to write something like
#define LLVM_ASM_PRINTER(TargetName) LLVMInitialize##TargetName##AsmPrinter();
to fix all targets. Attached patch adds defines only for for X86 and ARM, is it sufficient, should it be added for other targets or it can be completely skipped for LLVM < 2.8?
Patch also adds needed defines for LLVM Disassembler.

Also llvmpipe docs say that "LLVM: version 2.9 recommended; 2.6 or later required", is it up-to-date and build should also be tested with LLVM-2.6?

(In reply to comment #4)
> Created attachment 56422 [details] [review] [review]
> Proposed fix for LLVM-2.7
> 
> I don't really know how to properly fix this for all targets, because
> LLVM_NATIVE_ARCH in LLVM-2.7 is ${LLVM_NATIVE_ARCH}Target (X86Target for X86,
> ARMTarget for ARM etc), so it's not possible to write something like
> #define LLVM_ASM_PRINTER(TargetName) LLVMInitialize##TargetName##AsmPrinter();
> to fix all targets. Attached patch adds defines only for for X86 and ARM, is it
> sufficient, should it be added for other targets or it can be completely
> skipped for LLVM < 2.8?
> Patch also adds needed defines for LLVM Disassembler.

Thanks ojab.

> Also llvmpipe docs say that "LLVM: version 2.9 recommended; 2.6 or later
> required", is it up-to-date and build should also be tested with LLVM-2.6?

The ability to build with LLVM-2.6 is still useful for now, as it is one of the most stable LLVM releases for us ever.

But the ability to disassemble w/ LLVM 2.7 or eralier is not really important (as most developers will have recent LLVM anyway), so we could simply cut our losses and do:

--- a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
@@ -172,7 +172,7 @@ public:
 extern "C" void
 lp_disassemble(const void* func)
 {
-#if HAVE_LLVM >= 0x0207
+#if HAVE_LLVM >= 0x0208
    using namespace llvm;
 
    const uint8_t *bytes = (const uint8_t *)func;

Vinson, I don't have LLVM 2.7. Could you verify either ojab's or my change fixes the build issue?

(In reply to comment #5)
> 
> The ability to build with LLVM-2.6 is still useful for now, as it is one of the
> most stable LLVM releases for us ever.
> 
> But the ability to disassemble w/ LLVM 2.7 or eralier is not really important
> (as most developers will have recent LLVM anyway), so we could simply cut our
> losses and do:
> 
> --- a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
> +++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
> @@ -172,7 +172,7 @@ public:
>  extern "C" void
>  lp_disassemble(const void* func)
>  {
> -#if HAVE_LLVM >= 0x0207
> +#if HAVE_LLVM >= 0x0208
>     using namespace llvm;
> 
>     const uint8_t *bytes = (const uint8_t *)func;
> 
> Vinson, I don't have LLVM 2.7. Could you verify either ojab's or my change
> fixes the build issue?

The build failure in comment #3 doesn't occur with llvm-2.6. Only the build with llvm-2.7 is failing.

Jose, your patch fixes the build for llvm-2.7. The llvm-2.6 build is unaffected and still completes.

Tested-by: Vinson Lee <vlee@freedesktop.org>

(In reply to comment #4)
> Created attachment 56422 [details] [review] [review]
> Proposed fix for LLVM-2.7
> 
> I don't really know how to properly fix this for all targets, because
> LLVM_NATIVE_ARCH in LLVM-2.7 is ${LLVM_NATIVE_ARCH}Target (X86Target for X86,
> ARMTarget for ARM etc), so it's not possible to write something like
> #define LLVM_ASM_PRINTER(TargetName) LLVMInitialize##TargetName##AsmPrinter();
> to fix all targets. Attached patch adds defines only for for X86 and ARM, is it
> sufficient, should it be added for other targets or it can be completely
> skipped for LLVM < 2.8?
> Patch also adds needed defines for LLVM Disassembler.
> 
> Also llvmpipe docs say that "LLVM: version 2.9 recommended; 2.6 or later
> required", is it up-to-date and build should also be tested with LLVM-2.6?

ojab, your patch also fixes the build with llvm-2.7. The llvm-2.6 is unaffected and still completes.

Tested-by: Vinson Lee <vlee@freedesktop.org>

I pushed ojab's patch. Thanks Vinson and ojab.

This is broken. This is not valid C :-)

#if (LLVM_NATIVE_ARCH == X86 || LLVM_NATIVE_ARCH == X86Target)

Doesn't work with the C preprocessor. The preprocessor evaluates numbers,
but X86 and X86Target are not valid numbers (they aren't themselves
#defined to something).

So the above statement is equivalent to

#if (0 == 0 || 0 == 0)

Probably breaks the ARM build... (It does break on PPC which I'm playing with
right now but that's more than just that to it :-)

Should be fixed now in git.