[Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.

[Tom Stellard]

On Wed, Apr 17, 2013 at 07:54:32AM +0200, Mathias Fröhlich wrote:
> 
> Tom,
> 
> > -class LLVMEnsureMultithreaded {
> > -public:
> > -   LLVMEnsureMultithreaded()
> > -   {
> > -      llvm_start_multithreaded();
> > -   }
> > -};
> > -
> > -static LLVMEnsureMultithreaded lLVMEnsureMultithreaded;
> 
> Removing this leads to crashes in llvm with applications that concurrently 
> work on different gl contexts.

The test you wrote still passes with this patch.  Do you see that
we are now calling the C API version of llvm_start_multithreaded(),
LLVMStartMutithreaded() from inside radeon_llvm_compile() protected by a
static variable?

> 
> When I did this change I also provided a testcase that never made it into 
> piglit. See:
> http://lists.freedesktop.org/archives/piglit/2012-August/003100.html
> I had addresssed the concerns at that time with a v2 patch but got lost 
> without being checked in. May be somebody can push this now?
>

Sorry about that. I didn't have piglit commit access at the time, and
I forgot about the patch.  I fixed a few things and sent v3 to the list.
 
> Regarding the point where this funciton is called I had choosen static 
> initialization time since llvm requires this function to be called single 
> threaded which we cannot guarantee in any case. Keep in mind that you need to 
> ensure this function called non concurrently even against applications that 
> itself already use the llvm libs in some way while the driver is loaded.
> But the best bet is to do this in the dynamic loder which is itself 
> serialized, so I could avoid calling this function concurrently by 
> initialization of different contexts. That should at least shield against 
> applications that itself do the same trick by calling this funtion in the 
> dlopen phase in some static initializer ...
> We may get around part of this problem with dlopening the driver with better 
> isolation but up to now the problem can get that far.
> 

This is a tricky problem, and I'm not sure that radeon_llvm_compile() is
the best place to call llvm_start_multithreaded().  Maybe it would be
better to move this into gallivm, because this problem could affect any
driver that uses the gallivm code, which includes: llvmpipe, r300g, r600g,
radeonsi, and i915g.  What do you think?

-Tom

> You can find more thoughts about that problem regarding llvmpipe which still 
> has this problem is written down here:
> http://lists.freedesktop.org/archives/mesa-dev/2013-January/033015.html
> 
> So, a better place where you can switch llvm to be multithreaded is 
> apprecheated. But please make sure it happens at some point, the earlier, the 
> better.
> 
> If you want to have plain c files that behaves the same like what we have today 
> you can use a gcc extension (untested):
> 
> static void r600_llvm_init() __attribute__ ((__constructor__));
> void r600_llvm_init()
> {
>   llvm_start_multithreaded();
> }
> 
> Greetings
> 
> Mathias