[Mesa-dev] [PATCH] gallivm: correctly manage MCJIT at run-time

Emil Velikov emil.l.velikov at gmail.com
Mon Jan 16 13:46:30 UTC 2017


On 14 January 2017 at 08:46, Jose Fonseca <jfonseca at 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


More information about the mesa-dev mailing list