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.
Jose
On 14/01/17 01:31, Emil Velikov wrote:
Earlier commit made the decision whether to use MCJIT a run-time one. At
the same time, it changed the code-flow in the following manner:
- LLVMLinkInMCJIT() was executed regardless of whether MCJIT is to be
used or not. Admittedly it is a no-op at least in some builds.
- LLVMLinkInJIT() was executed regardless of weather MCJIT is to be
used or not.
Resolve that my promoting USE_MCJIT to be static bool, always. Make sure
it's honoured and the correct LLVMLinkIn{MC,}JIT() function is called
only as needed.
Fixes: cf4105740f0 "gallivm: Make MCJIT a runtime option."
Cc: Zhen Wu <wuz...@jidemail.com>
Cc: Jose Fonseca <jfons...@vmware.com>
Signed-off-by: Emil Velikov <emil.l.veli...@gmail.com>
---
Jose, rather than jumping around like a headless chicken I've went ahead
and fixed things... or maybe I broke it ? Afaict this preserves the
original behaviour and linking should be perfectly fine.
XXX: worth dropping the ALL_CAPS from the, now, variable name ? Should
we squash it here or as separate patch ?
As an added bonus might even solve the issue Wu Zhen is hitting :-)
---
src/gallium/auxiliary/gallivm/lp_bld_init.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.c
b/src/gallium/auxiliary/gallivm/lp_bld_init.c
index d1b2369f34..9a77c87ae4 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_init.c
+++ b/src/gallium/auxiliary/gallivm/lp_bld_init.c
@@ -45,9 +45,9 @@
/* Only MCJIT is available as of LLVM SVN r216982 */
#if HAVE_LLVM >= 0x0306
-# define USE_MCJIT 1
+static bool USE_MCJIT = 1;
#elif defined(PIPE_ARCH_PPC_64) || defined(PIPE_ARCH_S390) ||
defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
-# define USE_MCJIT 1
+static bool USE_MCJIT = 1;
#else
static bool USE_MCJIT = 0;
#endif
@@ -395,11 +395,11 @@ lp_build_init(void)
if (gallivm_initialized)
return TRUE;
- LLVMLinkInMCJIT();
-#if !defined(USE_MCJIT)
- USE_MCJIT = debug_get_bool_option("GALLIVM_MCJIT", 0);
- LLVMLinkInJIT();
-#endif
+ USE_MCJIT = debug_get_bool_option("GALLIVM_MCJIT", USE_MCJIT);
+ if (USE_MCJIT)
+ LLVMLinkInMCJIT();
+ else
+ LLVMLinkInJIT();
#ifdef DEBUG
gallivm_debug = debug_get_option_gallivm_debug();
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
