cc'ing the list this time.
On Wed, Aug 15, 2012 at 07:51:46AM +0200, Mathias Fröhlich wrote:
>
> Hi,
>
> This change adds application global locking around compiling an r600 llvm
> shader. This fixes a race condition/crash that I observe when shaders are
> concurrently compiled from different contexts in different threads.
> The fix is crude as it just guards the whole compile with a global mutex, but
> up to now I did not have enough time to understand the real reason that must
> be somewhere in the way r600g initializes the llvm backend. And since we have
> a pending public release we will better have a curde fix than a known bug.
> Better solutions welcome!
>
> I will post a piglit test in the next minutes that exercises this problem
> with
> a high probability at least here on my test machine.
>
> Please review
>
> Mathias
This is likely caused by the fact that the same LLVM Context is used
for all threads. Take a look at the comment starting at
src/gallium/auxilary/gallivm/lp_bld_init.c:314
You should be able to reproduce this error on llvmpipe as well.
Creating a new LLVM Context for each thread should fix this.
-Tom
> From 6501efdb1ef9f52b584a709c4d24e095b67fc91d Mon Sep 17 00:00:00 2001
> Message-Id:
> <6501efdb1ef9f52b584a709c4d24e095b67fc91d.1345008604.git.mathias.froehl...@gmx.net>
> From: =?UTF-8?q?Mathias=20Fr=C3=B6hlich?= <mathias.froehl...@gmx.net>
> Date: Fri, 10 Aug 2012 22:20:06 +0200
> Subject: [PATCH] radeon_llvm: Introduce global lock on llvm.
>
> No clue what is really racing here, but this fixes
> sporadic crashes running osgviewer with multiple
> windows together with the r600 llvm shader compiler.
>
> Signed-off-by: Mathias Froehlich <mathias.froehl...@web.de>
> ---
> src/gallium/drivers/radeon/radeon_llvm.h | 10 ++++++++
> src/gallium/drivers/radeon/radeon_llvm_emit.cpp | 14 +++++++++--
> .../drivers/radeon/radeon_setup_tgsi_llvm.c | 28
> ++++++++++++++++++++++
> 3 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/radeon_llvm.h
> b/src/gallium/drivers/radeon/radeon_llvm.h
> index 7a32bb0..d1b6d17 100644
> --- a/src/gallium/drivers/radeon/radeon_llvm.h
> +++ b/src/gallium/drivers/radeon/radeon_llvm.h
> @@ -142,6 +142,16 @@ static inline LLVMValueRef bitcast(
> return value;
> }
>
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +void radeon_llvm_lock(void);
> +void radeon_llvm_unlock(void);
> +
> +#ifdef __cplusplus
> +}
> +#endif
>
> void radeon_llvm_context_init(struct radeon_llvm_context * ctx);
>
> diff --git a/src/gallium/drivers/radeon/radeon_llvm_emit.cpp
> b/src/gallium/drivers/radeon/radeon_llvm_emit.cpp
> index 89130b3..39b4640 100644
> --- a/src/gallium/drivers/radeon/radeon_llvm_emit.cpp
> +++ b/src/gallium/drivers/radeon/radeon_llvm_emit.cpp
> @@ -48,15 +48,19 @@
>
> using namespace llvm;
>
> -#ifndef EXTERNAL_LLVM
> extern "C" {
>
> +#ifndef EXTERNAL_LLVM
> void LLVMInitializeAMDGPUTargetMC(void);
> void LLVMInitializeAMDGPUTarget(void);
> void LLVMInitializeAMDGPUTargetInfo(void);
> -}
> #endif
>
> +void radeon_llvm_lock();
> +void radeon_llvm_unlock();
> +
> +}
> +
> /**
> * Compile an LLVM module to machine code.
> *
> @@ -68,6 +72,8 @@ radeon_llvm_compile(LLVMModuleRef M, unsigned char ** bytes,
> unsigned * byte_count, const char * gpu_family,
> unsigned dump) {
>
> + radeon_llvm_lock();
> +
> Triple AMDGPUTriple(sys::getDefaultTargetTriple());
>
> #ifdef EXTERNAL_LLVM
> @@ -82,6 +88,7 @@ radeon_llvm_compile(LLVMModuleRef M, unsigned char ** bytes,
> std::string err;
> const Target * AMDGPUTarget = TargetRegistry::lookupTarget("r600", err);
> if(!AMDGPUTarget) {
> + radeon_llvm_unlock();
> fprintf(stderr, "Can't find target: %s\n", err.c_str());
> return 1;
> }
> @@ -119,6 +126,7 @@ radeon_llvm_compile(LLVMModuleRef M, unsigned char **
> bytes,
> /* Optional extra paramater true / false to disable verify */
> if (AMDGPUTargetMachine.addPassesToEmitFile(PM, out,
> TargetMachine::CGFT_AssemblyFile,
> true)){
> + radeon_llvm_unlock();
> fprintf(stderr, "AddingPasses failed.\n");
> return 1;
> }
> @@ -131,5 +139,7 @@ radeon_llvm_compile(LLVMModuleRef M, unsigned char **
> bytes,
> memcpy(*bytes, data.c_str(), data.length() * sizeof(unsigned char));
> *byte_count = data.length();
>
> + radeon_llvm_unlock();
> +
> return 0;
> }
> diff --git a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> index 641d277..95dd419 100644
> --- a/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> +++ b/src/gallium/drivers/radeon/radeon_setup_tgsi_llvm.c
> @@ -36,10 +36,26 @@
> #include "util/u_math.h"
> #include "util/u_memory.h"
> #include "util/u_debug.h"
> +#include "os/os_thread.h"
>
> #include <llvm-c/Core.h>
> #include <llvm-c/Transforms/Scalar.h>
>
> +/** llvm with its magnitudes of global variables and factories
> + * appears not to be thread safe. Until then, we sadly need a mutex.
> + */
> +pipe_static_mutex(llvm_mutex);
> +
> +void radeon_llvm_lock()
> +{
> + pipe_mutex_lock(llvm_mutex);
> +}
> +
> +void radeon_llvm_unlock()
> +{
> + pipe_mutex_unlock(llvm_mutex);
> +}
> +
> static struct radeon_llvm_loop * get_current_loop(struct radeon_llvm_context
> * ctx)
> {
> return ctx->loop_depth > 0 ? ctx->loop + (ctx->loop_depth - 1) : NULL;
> @@ -989,6 +1005,8 @@ void radeon_llvm_context_init(struct radeon_llvm_context
> * ctx)
> LLVMTypeRef main_fn_type;
> LLVMBasicBlockRef main_fn_body;
>
> + radeon_llvm_lock();
> +
> /* Initialize the gallivm object:
> * We are only using the module, context, and builder fields of this
> struct.
> * This should be enough for us to be able to pass our gallivm struct
> to the
> @@ -1166,11 +1184,16 @@ void radeon_llvm_context_init(struct
> radeon_llvm_context * ctx)
>
> bld_base->rsq_action.emit = build_tgsi_intrinsic_nomem;
> bld_base->rsq_action.intr_name = "llvm.AMDGPU.rsq";
> +
> + radeon_llvm_unlock();
> }
>
> void radeon_llvm_finalize_module(struct radeon_llvm_context * ctx)
> {
> struct gallivm_state * gallivm = ctx->soa.bld_base.base.gallivm;
> +
> + radeon_llvm_lock();
> +
> /* End the main function with Return*/
> LLVMBuildRetVoid(gallivm->builder);
>
> @@ -1191,10 +1214,15 @@ void radeon_llvm_finalize_module(struct
> radeon_llvm_context * ctx)
> LLVMDisposeBuilder(gallivm->builder);
> LLVMDisposePassManager(gallivm->passmgr);
>
> + radeon_llvm_unlock();
> }
>
> void radeon_llvm_dispose(struct radeon_llvm_context * ctx)
> {
> + radeon_llvm_lock();
> +
> LLVMDisposeModule(ctx->soa.bld_base.base.gallivm->module);
> LLVMContextDispose(ctx->soa.bld_base.base.gallivm->context);
> +
> + radeon_llvm_unlock();
> }
> --
> 1.7.11.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
