On 14 January 2017 at 08:46, Jose Fonseca <jfons...@vmware.com> wrote: > I suspect this might break builds with LLVM 3.6 or higher. > > The LLVMLinkInJIT must be inside #if ... #endif, and it must not be expanded > when HAVE_LLVM >= 0x0306, since LLVMLinkInJIT(), > > That is, when HAVE_LLVM >= 0x0306: > - USE_MCJIT should be static const > - no point claling GALLIVM_MCJIT > - must not have any LLVMLinkInJIT() call around, regardless it's called or > not. > > > And this code doesn't make sense: > > if (USE_MCJIT) > LLVMLinkInMCJIT(); > else > LLVMLinkInJIT(); > > If these functions are meant to force the static linking of external > libraries, putting any control flow around it is just misleading. It gives > the illusion that if we don't call these functions nothing will happen which > is defintely not true. > > >> As an added bonus might even solve the issue Wu Zhen is hitting :-) > > I'm not enterly sure I understad Wu Zhen problem. > > If android doesn't have MCJIT then I think the right fix is merely > > #if defined(ANDROID) > #define USE_MCJIT 0 > #endif > > ... > > > // Link MCJIT when USE_MCJIT is a runtime option or defined as 1 > #if !defined(USE_MCJIT) || USE_MCJIT > LLVMLinkInMCJIT(); > #endif > > // Link old JIT when USE_MCJIT is a runtime option or defined as 0 > #if !defined(USE_MCJIT) || !USE_MCJIT > LLVMLinkInJIT(); > #endif > > That is, any logic to decide whether to call or not LLVMLinkIn* must be done > with _build_ time C-processor logic. > I might be the only one here, but having FOO (USE_MCJIT in this case) as define or variable depending on $heuristics reads a bit iffy.
That aside - if I understood you correctly: - One of LLVMLinkIn{MC,}JIT might be missing on some versions of LLVM. In that case having a guard called "USE" sounds like a misnomer. Providing a static inline as needed might be cleaner ? - If LLVMLinkInJIT/LLVMLinkInMCJIT are solely for linking purposes, it will be better (imho) to add a small comment and still have them honour the user selection. With the latter, since we don't want things to explode as older/newer LLVM adds specific code in said functions. How does that sound ? Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev