Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi,
Sorry for these late response. I am in the middle of moving to a new
appartements coordinating work in the new one and so on.
It might even be that I do have no good network access for a few days ...
On Wednesday, May 01, 2013 17:56:33 Tom Stellard wrote:
> Thanks for pointing this out, I'll look into it. Do you know if there is
> any way to add C++ symbols to the EXPORT() command?
No, sorry.
To be honest I do not even know if the export command is the right way to go.
>From the documentation I had expected this to work slightly different, but in
the end it pulls these symbols and did what I need.
So, this is to work around the --whole-archive problem
If we can teach libtool to take the -Wl,-{no--,}whole-archive argument or
achieve a similar result, we can skip the EXPORT command. Also listing all the
llvm libs in the linker script was only to get the small proof of concept hack
together. It's probably better to take the llvm libraries that we get from the
configure step and use them on the command line instead of just squashing all
possible llvm libs in this GROUP line ...
> > > I didn't do much testing yet, but glxgears works for me with r600g and
> > > llvmpipe.
> >
> > I tried to test against the driver-isolation piglit test that I sent. But
> > this still fails.
> >
> > So, this simplest and least intrusive variant appears not to work as
> > expected. Not sure what the exact reason is:
> > Either, non versioned symbols take precedence or you cannot shield symbols
> > from different libraries by symbol versions. Keep in mind that this
> > mechanism was invented to maintain backwards compatibility for functions
> > that changed in a newer libc library in a api incompatible way, but still
> > provide the old variant for executables/libraries that linked against an
> > older libc. So what remains is either dig deeper there, or we use the
> > much more intrusive explicit Mesa prefix to the C api functions. The
> > direct c++ use is still unsolved then.
>
> Today, I spent some time on this and implemented your solution using the
> MesaLLVM prefix on all LLVM functions. The branch is here:
> http://cgit.freedesktop.org/~tstellar/mesa/log/?h=libmesallvm-prefix
>
> It's pretty hacky right now. Only the r600 driver will build, and I've
> commented all the c++ code out of gallivm. However, it passes your
> driver-isolation test and even works with clover as long as clover
> links to the stock libLLVM-3.3.so library.
Ok, that sounds good so far.
> We still need to figure out what to do about he c++ usage. I think some
> of it can be replaced with LLVM C API calls, but we may need to use our own
> C wrappers for some of the code.
Yes, I think so.
So, for example, lp_disassemble() would move into something like
MesaLLVMDisassemble() and lp_bld_debug.cpp would pull the MesaLLVMDisassemble
function?
Jose, does this fit your plans?
> It seems like the prefixing the LLVM calls may be the way to go, let me
> know if you have any suggestions for improvements.
Not more ideas than are already floating around I think.
Thank you very much for doing all this!
Greetings
Mathias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Wed, May 01, 2013 at 04:11:51PM +0200, Mathias Fröhlich wrote: > > Tom, Jose, > > On Tuesday, April 30, 2013 16:56:56 Tom Stellard wrote: > > I took the linker script from your email and took at shot at creating > > libMesaLLVM.so within Mesa. I've pushed my initial code here: > > http://cgit.freedesktop.org/~tstellar/mesa/log/?h=libmesallvm > Thank you very much and sorry for the late reply. > > > I ran into a few minor issues: > > > > I had to export all the LLVM symbols in libMesaLLVM.so, because gallivm > > still uses some C++ functions, and I was unsure how to handle the name > > mangling in the linker script. > You can even add c++ symbols directly there. See > http://sourceware.org/binutils/docs/ld/VERSION.html > Thanks for pointing this out, I'll look into it. Do you know if there is any way to add C++ symbols to the EXPORT() command? > > Clover still has a number of undefined symbols. I'm still not quite > > sure what the problem is, but I think the problem has something > > to do with the LLVM symbols in the clang libraries clover is using. > Also not sure, but I assume that some .o files from the .a libs are not > pulled > because they are unused by the C api. The C api is explicitly pulled in the > linker script. Which works around the lack of --whole-archive in the linker > script. So, probably these missing symbols do just not end up in the shared > library. > Yeah, this was the problem, clang needed symbols that weren't used by the C API, so they weren't included in libMesaLLVM.so. > > I didn't do much testing yet, but glxgears works for me with r600g and > > llvmpipe. > I tried to test against the driver-isolation piglit test that I sent. But > this > still fails. > > So, this simplest and least intrusive variant appears not to work as expected. > Not sure what the exact reason is: > Either, non versioned symbols take precedence or you cannot shield symbols > from different libraries by symbol versions. Keep in mind that this mechanism > was invented to maintain backwards compatibility for functions that changed > in > a newer libc library in a api incompatible way, but still provide the old > variant for executables/libraries that linked against an older libc. > So what remains is either dig deeper there, or we use the much more intrusive > explicit Mesa prefix to the C api functions. The direct c++ use is still > unsolved then. > Today, I spent some time on this and implemented your solution using the MesaLLVM prefix on all LLVM functions. The branch is here: http://cgit.freedesktop.org/~tstellar/mesa/log/?h=libmesallvm-prefix It's pretty hacky right now. Only the r600 driver will build, and I've commented all the c++ code out of gallivm. However, it passes your driver-isolation test and even works with clover as long as clover links to the stock libLLVM-3.3.so library. We still need to figure out what to do about he c++ usage. I think some of it can be replaced with LLVM C API calls, but we may need to use our own C wrappers for some of the code. It seems like the prefixing the LLVM calls may be the way to go, let me know if you have any suggestions for improvements. -Tom > > Also, note that there are 4 new commits in that repo, the first three > > are just variations from my previous C API patches for drivers/radeon. > > The biggest change is that I moved the static initializer that calls > > the llvm_multithreaded* functions into gallivm/lp_bld_misc.cpp > From my point of view this part is fine. > > > Let me know if you have any questions, concerns or other ideas. > Currently not. > Thank you for integrating that all and trying that all out! > > Greetings > > Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Tom, Jose, On Tuesday, April 30, 2013 16:56:56 Tom Stellard wrote: > I took the linker script from your email and took at shot at creating > libMesaLLVM.so within Mesa. I've pushed my initial code here: > http://cgit.freedesktop.org/~tstellar/mesa/log/?h=libmesallvm Thank you very much and sorry for the late reply. > I ran into a few minor issues: > > I had to export all the LLVM symbols in libMesaLLVM.so, because gallivm > still uses some C++ functions, and I was unsure how to handle the name > mangling in the linker script. You can even add c++ symbols directly there. See http://sourceware.org/binutils/docs/ld/VERSION.html > Clover still has a number of undefined symbols. I'm still not quite > sure what the problem is, but I think the problem has something > to do with the LLVM symbols in the clang libraries clover is using. Also not sure, but I assume that some .o files from the .a libs are not pulled because they are unused by the C api. The C api is explicitly pulled in the linker script. Which works around the lack of --whole-archive in the linker script. So, probably these missing symbols do just not end up in the shared library. > I didn't do much testing yet, but glxgears works for me with r600g and > llvmpipe. I tried to test against the driver-isolation piglit test that I sent. But this still fails. So, this simplest and least intrusive variant appears not to work as expected. Not sure what the exact reason is: Either, non versioned symbols take precedence or you cannot shield symbols from different libraries by symbol versions. Keep in mind that this mechanism was invented to maintain backwards compatibility for functions that changed in a newer libc library in a api incompatible way, but still provide the old variant for executables/libraries that linked against an older libc. So what remains is either dig deeper there, or we use the much more intrusive explicit Mesa prefix to the C api functions. The direct c++ use is still unsolved then. > Also, note that there are 4 new commits in that repo, the first three > are just variations from my previous C API patches for drivers/radeon. > The biggest change is that I moved the static initializer that calls > the llvm_multithreaded* functions into gallivm/lp_bld_misc.cpp >From my point of view this part is fine. > Let me know if you have any questions, concerns or other ideas. Currently not. Thank you for integrating that all and trying that all out! Greetings Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Sat, Apr 27, 2013 at 10:33:29AM +0200, Mathias Fröhlich wrote: > > Hi, > > On Thursday, April 25, 2013 10:29:27 Jose Fonseca wrote: > > - There are a bunch of options that need to be set via globals, (see > > lp_set_target_options), so app/drivers could tamper with each other > > options. > > > > - llvm::cl::ParseCommandLineOptions will complain if called multiple times > > -- I think we no longer need to call it these days though > > > > In short, LLVM was not designed for multiple users in the same process. > Yep. > > Also llvm is still emerging too fast to assume a specific version to be > available. At least with r600 we do currently need a somewhat recent version > and kind of have this assumption. > But due to the api not kept strictly backwards compatible and all the > pitfalls > that happen while emerging fast its very likely that a potential application > that also tries to make use of the driver modules just brings its own > probably > incompatible llvm version in some way. So shielding this in any way makes > sense ... > > > For the Mesa wrappers: > I have attached a shell script again as a rapid proof that is able to build a > linker script that builds up a wrapper shared library that contains a private > llvm copy. That's again non optimal - it contains just all static libs that I > have in my current test environment... It's just to sketch how this could > work. > > The MesaLLVM-with-prefix.link script can by used with the command > > g++ -shared -o libMesaLLVM.so MesaLLVM-with-prefix.link > > to produce a libMesaLLVM.so that contains all C symbols starting with LLVM > from libLLVMCore.a. All of them get prefixed with Mesa and are the only > exported symbols then. > That's close to Joses suggestion but with less work to be done in sources. > > The MesaLLVM-with-version.link script can by used with the command > > g++ -shared -o libMesaLLVM.so MesaLLVM-with-version.link > > to produce a libMesaLLVM.so shared library that uses symbol versioning to > distinguish between the llvm versions. I got this idea yesterday and this > might simplify the problem a lot. > By this variant we do not even need to prefix all the callers by Mesa. What > this does is to explicitly assign a symbol version to all these calls. At > static link symbol resolve time with this libMesaLLVM.so, this symbol version > > (the 'A MesaLLVM_1.0' entry) is then pulled out of this shared object and all > users, in libllvmradeon.so for example, are linked against > LLVMCreateContext@MesaLLVM_1,0 instead of just LLVMCreateContext. So we > should > get a private copy of llvm in libMesaLLVM.so with just the same call names > than usual source code wise. > That's to be tested and verified, but if this works like I think it should, > this is the easiest way to get our own LLVM version on linux at least. > > And sorry for just doing these crude proof of concept stuff ... > Hi Mathias, I took the linker script from your email and took at shot at creating libMesaLLVM.so within Mesa. I've pushed my initial code here: http://cgit.freedesktop.org/~tstellar/mesa/log/?h=libmesallvm I ran into a few minor issues: I had to export all the LLVM symbols in libMesaLLVM.so, because gallivm still uses some C++ functions, and I was unsure how to handle the name mangling in the linker script. Clover still has a number of undefined symbols. I'm still not quite sure what the problem is, but I think the problem has something to do with the LLVM symbols in the clang libraries clover is using. I didn't do much testing yet, but glxgears works for me with r600g and llvmpipe. Also, note that there are 4 new commits in that repo, the first three are just variations from my previous C API patches for drivers/radeon. The biggest change is that I moved the static initializer that calls the llvm_multithreaded* functions into gallivm/lp_bld_misc.cpp Let me know if you have any questions, concerns or other ideas. Thanks, Tom > /* Mesa llvm linker script */ > EXTERN( > LLVMAddAlias > LLVMAddAttribute > LLVMAddCase > LLVMAddClause > LLVMAddDestination > LLVMAddFunction > LLVMAddFunctionAttr > LLVMAddGlobal > LLVMAddGlobalInAddressSpace > LLVMAddIncoming > LLVMAddInstrAttribute > LLVMAddNamedMetadataOperand > LLVMAddTargetDependentFunctionAttr > LLVMAlignOf > LLVMAppendBasicBlock > LLVMAppendBasicBlockInContext > LLVMArrayType > LLVMBasicBlockAsValue > LLVMBlockAddress > LLVMBuildAdd > LLVMBuildAggregateRet > LLVMBuildAlloca > LLVMBuildAnd > LLVMBuildArrayAlloca > LLVMBuildArrayMalloc > LLVMBuildAShr > LLVMBuildAtomicRMW > LLVMBuildBinOp > LLVMBuildBitCast > LLVMBuildBr > LLVMBuildCall > LLVMBuildCast > LLVMBuildCondBr > LLVMBuildExactSDiv > LLVMBuildExtractElement > LLVMBuildExtractValue > LLVMBuildFAdd > LLVMBuildFCmp > LLVMBuildFDiv > LLVMBuildFMul > LLVMBuildFNeg > LLVMBuildFPCast > LLVMBuildFPExt > LLVMBuildFPToSI > LLVMBuildFPToUI > LLVMBuildFPTrunc > LLVMBuildFree > LLVMBuildFRem > LLVMBuildFSub > LLVMBuildGE
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
- Original Message - > > Hi, > > On Thursday, April 25, 2013 10:29:27 Jose Fonseca wrote: > > - There are a bunch of options that need to be set via globals, (see > > lp_set_target_options), so app/drivers could tamper with each other > > options. > > > > - llvm::cl::ParseCommandLineOptions will complain if called multiple times > > -- I think we no longer need to call it these days though > > > > In short, LLVM was not designed for multiple users in the same process. > Yep. > > Also llvm is still emerging too fast to assume a specific version to be > available. At least with r600 we do currently need a somewhat recent version > and kind of have this assumption. > But due to the api not kept strictly backwards compatible and all the > pitfalls > that happen while emerging fast its very likely that a potential application > that also tries to make use of the driver modules just brings its own > probably > incompatible llvm version in some way. So shielding this in any way makes > sense ... > > > For the Mesa wrappers: > I have attached a shell script again as a rapid proof that is able to build a > linker script that builds up a wrapper shared library that contains a private > llvm copy. > > That's again non optimal - it contains just all static libs that I > have in my current test environment... It's just to sketch how this could > work. > > The MesaLLVM-with-prefix.link script can by used with the command > > g++ -shared -o libMesaLLVM.so MesaLLVM-with-prefix.link > > to produce a libMesaLLVM.so that contains all C symbols starting with LLVM > from libLLVMCore.a. All of them get prefixed with Mesa and are the only > exported symbols then. > That's close to Joses suggestion but with less work to be done in sources. > > The MesaLLVM-with-version.link script can by used with the command > > g++ -shared -o libMesaLLVM.so MesaLLVM-with-version.link > > to produce a libMesaLLVM.so shared library that uses symbol versioning to > distinguish between the llvm versions. I got this idea yesterday and this > might simplify the problem a lot. > By this variant we do not even need to prefix all the callers by Mesa. What > this does is to explicitly assign a symbol version to all these calls. At > static link symbol resolve time with this libMesaLLVM.so, this symbol version > (the 'A MesaLLVM_1.0' entry) is then pulled out of this shared object and all > users, in libllvmradeon.so for example, are linked against > LLVMCreateContext@MesaLLVM_1,0 instead of just LLVMCreateContext. So we > should > get a private copy of llvm in libMesaLLVM.so with just the same call names > than usual source code wise. > That's to be tested and verified, but if this works like I think it should, > this is the easiest way to get our own LLVM version on linux at least. Wow. I confess I wasn't aware that linker scripts were this powerful. And if it works, using a custom version instead of a custom symbol prefix seems a genial idea IMO. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi, On Thursday, April 25, 2013 10:29:27 Jose Fonseca wrote: > - There are a bunch of options that need to be set via globals, (see > lp_set_target_options), so app/drivers could tamper with each other > options. > > - llvm::cl::ParseCommandLineOptions will complain if called multiple times > -- I think we no longer need to call it these days though > > In short, LLVM was not designed for multiple users in the same process. Yep. Also llvm is still emerging too fast to assume a specific version to be available. At least with r600 we do currently need a somewhat recent version and kind of have this assumption. But due to the api not kept strictly backwards compatible and all the pitfalls that happen while emerging fast its very likely that a potential application that also tries to make use of the driver modules just brings its own probably incompatible llvm version in some way. So shielding this in any way makes sense ... For the Mesa wrappers: I have attached a shell script again as a rapid proof that is able to build a linker script that builds up a wrapper shared library that contains a private llvm copy. That's again non optimal - it contains just all static libs that I have in my current test environment... It's just to sketch how this could work. The MesaLLVM-with-prefix.link script can by used with the command g++ -shared -o libMesaLLVM.so MesaLLVM-with-prefix.link to produce a libMesaLLVM.so that contains all C symbols starting with LLVM from libLLVMCore.a. All of them get prefixed with Mesa and are the only exported symbols then. That's close to Joses suggestion but with less work to be done in sources. The MesaLLVM-with-version.link script can by used with the command g++ -shared -o libMesaLLVM.so MesaLLVM-with-version.link to produce a libMesaLLVM.so shared library that uses symbol versioning to distinguish between the llvm versions. I got this idea yesterday and this might simplify the problem a lot. By this variant we do not even need to prefix all the callers by Mesa. What this does is to explicitly assign a symbol version to all these calls. At static link symbol resolve time with this libMesaLLVM.so, this symbol version (the 'A MesaLLVM_1.0' entry) is then pulled out of this shared object and all users, in libllvmradeon.so for example, are linked against LLVMCreateContext@MesaLLVM_1,0 instead of just LLVMCreateContext. So we should get a private copy of llvm in libMesaLLVM.so with just the same call names than usual source code wise. That's to be tested and verified, but if this works like I think it should, this is the easiest way to get our own LLVM version on linux at least. And sorry for just doing these crude proof of concept stuff ... Greetings Mathias mklinkerscript.sh Description: application/shellscript /* Mesa llvm linker script */ EXTERN( LLVMAddAlias LLVMAddAttribute LLVMAddCase LLVMAddClause LLVMAddDestination LLVMAddFunction LLVMAddFunctionAttr LLVMAddGlobal LLVMAddGlobalInAddressSpace LLVMAddIncoming LLVMAddInstrAttribute LLVMAddNamedMetadataOperand LLVMAddTargetDependentFunctionAttr LLVMAlignOf LLVMAppendBasicBlock LLVMAppendBasicBlockInContext LLVMArrayType LLVMBasicBlockAsValue LLVMBlockAddress LLVMBuildAdd LLVMBuildAggregateRet LLVMBuildAlloca LLVMBuildAnd LLVMBuildArrayAlloca LLVMBuildArrayMalloc LLVMBuildAShr LLVMBuildAtomicRMW LLVMBuildBinOp LLVMBuildBitCast LLVMBuildBr LLVMBuildCall LLVMBuildCast LLVMBuildCondBr LLVMBuildExactSDiv LLVMBuildExtractElement LLVMBuildExtractValue LLVMBuildFAdd LLVMBuildFCmp LLVMBuildFDiv LLVMBuildFMul LLVMBuildFNeg LLVMBuildFPCast LLVMBuildFPExt LLVMBuildFPToSI LLVMBuildFPToUI LLVMBuildFPTrunc LLVMBuildFree LLVMBuildFRem LLVMBuildFSub LLVMBuildGEP LLVMBuildGlobalString LLVMBuildGlobalStringPtr LLVMBuildICmp LLVMBuildInBoundsGEP LLVMBuildIndirectBr LLVMBuildInsertElement LLVMBuildInsertValue LLVMBuildIntCast LLVMBuildIntToPtr LLVMBuildInvoke LLVMBuildIsNotNull LLVMBuildIsNull LLVMBuildLandingPad LLVMBuildLoad LLVMBuildLShr LLVMBuildMalloc LLVMBuildMul LLVMBuildNeg LLVMBuildNot LLVMBuildNSWAdd LLVMBuildNSWMul LLVMBuildNSWNeg LLVMBuildNSWSub LLVMBuildNUWAdd LLVMBuildNUWMul LLVMBuildNUWNeg LLVMBuildNUWSub LLVMBuildOr LLVMBuildPhi LLVMBuildPointerCast LLVMBuildPtrDiff LLVMBuildPtrToInt LLVMBuildResume LLVMBuildRet LLVMBuildRetVoid LLVMBuildSDiv LLVMBuildSelect LLVMBuildSExt LLVMBuildSExtOrBitCast LLVMBuildShl LLVMBuildShuffleVector LLVMBuildSIToFP LLVMBuildSRem LLVMBuildStore LLVMBuildStructGEP LLVMBuildSub LLVMBuildSwitch LLVMBuildTrunc LLVMBuildTruncOrBitCast LLVMBuildUDiv LLVMBuildUIToFP LLVMBuildUnreachable LLVMBuildURem LLVMBuildVAArg LLVMBuildXor LLVMBuildZExt LLVMBuildZExtOrBitCast LLVMClearInsertionPosition LLVMConstAdd LLVMConstAllOnes LLVMConstAnd LLVMConstArray LLVMConstAShr LLVMConstBitCast LLVMConstExactSDiv LLVMConstExtractElement LLVMConstExtractValue LLVMConstFAdd LLVMConstFCmp LLVMConstFDiv LLVMConstFMul LLVMConstFNeg LLVMConstFPCast LLVMConstFPExt LLVMConstFPToSI LLVMConstFPToUI LLVMConstF
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Jose, On Thursday, April 25, 2013 03:52:44 Jose Fonseca wrote: > There is no paradox. To be clear: Ok, thanks! Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
- Original Message - > On Wed, Apr 24, 2013 at 09:54:02PM -0700, Jose Fonseca wrote: > > - Original Message - > > > On Wed, Apr 24, 2013 at 09:40:44PM +0200, Mathias Fröhlich wrote: > > > > > > > > Hi Tom, > > > > > > > > On Tuesday, April 23, 2013 20:47:24 Tom Stellard wrote: > > > > > First of all, thanks for investigating this. The information you've > > > > > provided has helped me a lot. > > > > Good to hear that it helps. > > > > > > > > > I took a shot at implementing it this way with private static copies > > > > > of > > > > > llvm. I've pushed the initial work to this branch: > > > > > git://people.freedesktop.org/~tstellar/mesa llvm-build > > > > > > > > > > I was able to hide the LLVM symbols by using the --exclude-libs > > > > > linker > > > > > flag and so far I've tested with glxgears, an opencl example and also > > > > > eglgears and they all work. I still need to do some more testing, > > > > > but > > > > > any thoughts on what I've done so far in this branch? > > > > > > > > I will look at this as soon as I find time. Probably not before the > > > > beginning > > > > of next week. > > > > > > > > > > I've thought about this some more, and I think that the best solution > > > might be to move all LLVM API calls into gallivm and build it as a > > > shared object with it's own private copy of LLVM statically linked. This > > > way > > > we > > > would still have a private copy of LLVM to work with, but without having > > > to statically link every single gallium target with LLVM. Any thoughts > > > about this? > > > > I don't see how this would work -- llvmpipe/draw has LLVMBuildXxxx calls > > too. So to prevent symbol collision with apps that use them, we'd need to > > expose all LLVM calls we need under nome unique prefix. > > > > I didn't realize this, and actually the radeon drivers use a lot of > the LLVMBuildXxxx calls too. > > > Also note that gallivm has a lot of function calls. This would also pollute > > the app's namespace. > > > > In short, for this to work we should do it not for whole gallivm, but with > > a new module, just for LLVM C calls + the additional helpers, and use an > > unique prefix (e.g., MesaLLVMXxxx) for everything. > > > > And we should have a new header, that #defines LLVMFoo MesaLLVMFoo, so that > > Mesa code doesn't need to be changed all over the place. > > > > I think this is a good approach, however before we make big changes to > Mesa I want to make sure we know what problems we are trying to solve > with these changes. As far as I understand it, the current problems > are: > > 1. If an application is using the LLVM API outside of Mesa, then it is > impossible for Mesa to safely use the llvm_*_multithreaded() functions. > > 2. If an application is using a different version of LLVM than Mesa, > then it might incorrectly use the LLVM symbols that are included in > the Mesa libraries. > > Are there others? I know a couple more: - There are a bunch of options that need to be set via globals, (see lp_set_target_options), so app/drivers could tamper with each other options. - llvm::cl::ParseCommandLineOptions will complain if called multiple times -- I think we no longer need to call it these days though In short, LLVM was not designed for multiple users in the same process. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Am 25.04.2013 19:08, schrieb Tom Stellard: [SNIP] I think this is a good approach, however before we make big changes to Mesa I want to make sure we know what problems we are trying to solve with these changes. As far as I understand it, the current problems are: 1. If an application is using the LLVM API outside of Mesa, then it is impossible for Mesa to safely use the llvm_*_multithreaded() functions. 2. If an application is using a different version of LLVM than Mesa, then it might incorrectly use the LLVM symbols that are included in the Mesa libraries. Are there others? Loading both r600g and radeonsi with the llvm backend into one process. Those two might want to use different versions of LLVM at the same time, so we might want to abstract the interface even further to the dri level, but not sure about that. Christian. -Tom ___ 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
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Wed, Apr 24, 2013 at 09:54:02PM -0700, Jose Fonseca wrote: > - Original Message - > > On Wed, Apr 24, 2013 at 09:40:44PM +0200, Mathias Fröhlich wrote: > > > > > > Hi Tom, > > > > > > On Tuesday, April 23, 2013 20:47:24 Tom Stellard wrote: > > > > First of all, thanks for investigating this. The information you've > > > > provided has helped me a lot. > > > Good to hear that it helps. > > > > > > > I took a shot at implementing it this way with private static copies of > > > > llvm. I've pushed the initial work to this branch: > > > > git://people.freedesktop.org/~tstellar/mesa llvm-build > > > > > > > > I was able to hide the LLVM symbols by using the --exclude-libs linker > > > > flag and so far I've tested with glxgears, an opencl example and also > > > > eglgears and they all work. I still need to do some more testing, but > > > > any thoughts on what I've done so far in this branch? > > > > > > I will look at this as soon as I find time. Probably not before the > > > beginning > > > of next week. > > > > > > > I've thought about this some more, and I think that the best solution > > might be to move all LLVM API calls into gallivm and build it as a > > shared object with it's own private copy of LLVM statically linked. This > > way > > we > > would still have a private copy of LLVM to work with, but without having > > to statically link every single gallium target with LLVM. Any thoughts > > about this? > > I don't see how this would work -- llvmpipe/draw has LLVMBuildXxxx calls too. > So to prevent symbol collision with apps that use them, we'd need to expose > all LLVM calls we need under nome unique prefix. > I didn't realize this, and actually the radeon drivers use a lot of the LLVMBuildXxxx calls too. > Also note that gallivm has a lot of function calls. This would also pollute > the app's namespace. > > In short, for this to work we should do it not for whole gallivm, but with a > new module, just for LLVM C calls + the additional helpers, and use an unique > prefix (e.g., MesaLLVMXxxx) for everything. > > And we should have a new header, that #defines LLVMFoo MesaLLVMFoo, so that > Mesa code doesn't need to be changed all over the place. > I think this is a good approach, however before we make big changes to Mesa I want to make sure we know what problems we are trying to solve with these changes. As far as I understand it, the current problems are: 1. If an application is using the LLVM API outside of Mesa, then it is impossible for Mesa to safely use the llvm_*_multithreaded() functions. 2. If an application is using a different version of LLVM than Mesa, then it might incorrectly use the LLVM symbols that are included in the Mesa libraries. Are there others? -Tom ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
- Original Message -
>
> Jose,
>
> On Thursday, April 25, 2013 01:38:46 Jose Fonseca wrote:
> > What I'm suggesting doesn't require huge effort.
> >
> > In detail, I'm suggesting:
> >
> > (1) have a custom build of LLVM libraries with -fvisibility=hidden
> >
> > (2) have a src/mesallvm/mesallvm.c containing wrappers
> >
> > #include
> >
> > __attribute__((visibility("default")))
> > LLVMValueRef
> > MesaLLVMBuildAdd(LLVMBuilderRef Builder, LLVMValueRef LHS, LLVMValueRef
> > RHS, const char *Name) { return LLVMBuildAdd(Builder, LHS, RHS, Name);
> > }
> >
> > ...
> >
> > plus a src/mesallvm/mesallvm.cpp with whatever custom C wrappers to C++
> > functionality we need (e.g, the stuff in lp_build_misc.cpp).
> >
> > These two modules would be statically linked against the custom LLVM
> > libraries from (1), producing a shared library libMESALLVM.so, which would
> > be consumed by any Mesa driver that needs it. Because all symbols have the
> > unique MESALLVM prefix, these should not collide with user stuff.
> >
> > (3) have a src/mesallvm/mesallvm.h
> >
> >// Mange LLVM symbols before including them
> >#define LLVMBuildAdd MesaLLVMBuildAdd
> >
> >...
> >
> >#include
> >...
> >
> >All gallium code would include src/mesallvm/mesallvm.h instead. It can
> > still use the LLVMFoo as befor.
> >
> > And that's all there is to it.
>
> Technically this will work.
>
> But I fear that this build procedure is too non standard to be really picked
> up by the distribution builders. They would need to build a seperate llvm
> version inside of the mesa package. Doable, but do you think we can convince
> them all?
This is for you and them to decide. I have no experience nor opinion on that.
> The technical problem you try to solve is that we need to force the llvm
> build
> to have the default hidden visibility.
> I think we could achieve the same result by using a default built llvm static
> lib and explicitly mention the to be exported symbols in a linker script.
> IIRC
> we could even use wildcards there. That is only export everything starting
> with MesaLLVM*.
>
> Remains that we need to write out all the wrappers, which I called 'huge
> effort'.
I think our scales are greatness are too different:
- apitrace has to deal with 2861 entry points
- LLVM C bindings has 588
$ grep -ro '\ May be this could equally well done by looking with scripts at the llvm
> libraries and create an apropriate version script. Hide all symbols, except
> the ones starting with MesaLLVM and for each LLVM c symbol create an alias
> with the apropriate MesaLLVM symbol. Requires some shell or python hackery to
> generate the version script given the llvm c binding library.
If it works, sounds great by me. Feel free to ignore my suggestion.
My main concern is foremost what happens gallivm, and not so much the symbol
problem. The motivation of my suggestions was merely to provide alternatives to
those I foresee problematic.
> > > Again OTOH, the dlopen flags are really made for this kind of module use
> > > I
> > > think ...
>
> I was referencing the patches that i just packed into the mail yesterday.
> This
> RTLD_LOCAL|RTLD_DEEPBIND flag already does what we need regarding isolation.
> This does not even require more changes than changing a bunch of dlopen flags
> -
> no wrappers to program.
> What I am not sure about is if we isolate too much for mesa/dri internal
> needs
> with this.
I confess I haven't had opportunity to look at them yet. I'm not familiar with
the semantics of RTLD_DEEPBIND neither, so I can't comment.
> > Nevertheless, I must insist that the problem of hiding LLVM symbols is
> > handled in a layer underneath gallivm. Severing gallivm module from the
> > rest auxiliary gallium modules, or making gallivm a full wrapper for all
> > LLVM symbols, seems unsustainable / too limiting IMO.
> Huch, then I probably do not get you right.
> Aren't you proposing a full wrapper of all the llvm c symbols by MesaLLVM*
> symbols above? Here you are calling exactly the same too limiting?
There is no paradox. To be clear:
- Severing gallivm from rest of auxiliary modules is too limiting -- gallivm
needs those modules. The wrappers I recommmended are self sufficient. They are
in no way limited by being isolated from other gallium auxiliary modules.
- making gallivm a wrapper for all symbols is unsustainable, ugly. Take a look
at gallivm abstraction for add operation:
/**
* Generate a + b
*/
LLVMValueRef
lp_build_add(struct lp_build_context *bld,
LLVMValueRef a,
LLVMValueRef b)
{
LLVMBuilderRef builder = bld->gallivm->builder;
const struct lp_type type = bld->type;
LLVMValueRef res;
assert(lp_check_value(type, a));
assert(lp_check_value(type, b));
if(a == bld->zero)
return b;
if(b == bld->zero)
return a;
if(a == bld->undef || b == bld->undef)
return bld->undef;
if(bld->type.norm)
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Jose,
On Thursday, April 25, 2013 01:38:46 Jose Fonseca wrote:
> What I'm suggesting doesn't require huge effort.
>
> In detail, I'm suggesting:
>
> (1) have a custom build of LLVM libraries with -fvisibility=hidden
>
> (2) have a src/mesallvm/mesallvm.c containing wrappers
>
> #include
>
> __attribute__((visibility("default")))
> LLVMValueRef
> MesaLLVMBuildAdd(LLVMBuilderRef Builder, LLVMValueRef LHS, LLVMValueRef
> RHS, const char *Name) { return LLVMBuildAdd(Builder, LHS, RHS, Name);
> }
>
> ...
>
> plus a src/mesallvm/mesallvm.cpp with whatever custom C wrappers to C++
> functionality we need (e.g, the stuff in lp_build_misc.cpp).
>
> These two modules would be statically linked against the custom LLVM
> libraries from (1), producing a shared library libMESALLVM.so, which would
> be consumed by any Mesa driver that needs it. Because all symbols have the
> unique MESALLVM prefix, these should not collide with user stuff.
>
> (3) have a src/mesallvm/mesallvm.h
>
>// Mange LLVM symbols before including them
>#define LLVMBuildAdd MesaLLVMBuildAdd
>
>...
>
>#include
>...
>
>All gallium code would include src/mesallvm/mesallvm.h instead. It can
> still use the LLVMFoo as befor.
>
> And that's all there is to it.
Technically this will work.
But I fear that this build procedure is too non standard to be really picked
up by the distribution builders. They would need to build a seperate llvm
version inside of the mesa package. Doable, but do you think we can convince
them all?
The technical problem you try to solve is that we need to force the llvm build
to have the default hidden visibility.
I think we could achieve the same result by using a default built llvm static
lib and explicitly mention the to be exported symbols in a linker script. IIRC
we could even use wildcards there. That is only export everything starting
with MesaLLVM*.
Remains that we need to write out all the wrappers, which I called 'huge
effort'.
May be this could equally well done by looking with scripts at the llvm
libraries and create an apropriate version script. Hide all symbols, except
the ones starting with MesaLLVM and for each LLVM c symbol create an alias
with the apropriate MesaLLVM symbol. Requires some shell or python hackery to
generate the version script given the llvm c binding library.
> > Is it possible to hide all llvm calls behind the lp_bld_* interface or
> > similar
> > and put this layer into a single shared object statically linked with all
> > of llvm?
>
> No, gallivm was not intended a full abstration to LLVM C bindings,
> furthermore it is not self-contained (it depends on lots of stuff from
> other auxiliary modules).
Ok.
> > Again OTOH, the dlopen flags are really made for this kind of module use I
> > think ...
I was referencing the patches that i just packed into the mail yesterday. This
RTLD_LOCAL|RTLD_DEEPBIND flag already does what we need regarding isolation.
This does not even require more changes than changing a bunch of dlopen flags -
no wrappers to program.
What I am not sure about is if we isolate too much for mesa/dri internal needs
with this.
> Nevertheless, I must insist that the problem of hiding LLVM symbols is
> handled in a layer underneath gallivm. Severing gallivm module from the
> rest auxiliary gallium modules, or making gallivm a full wrapper for all
> LLVM symbols, seems unsustainable / too limiting IMO.
Huch, then I probably do not get you right.
Aren't you proposing a full wrapper of all the llvm c symbols by MesaLLVM*
symbols above? Here you are calling exactly the same too limiting?
Greetings
Mathias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
- Original Message -
>
> Hi,
>
> On Wednesday, April 24, 2013 21:54:02 Jose Fonseca wrote:
> > I don't see how this would work -- llvmpipe/draw has LLVMBuildXxxx calls
> > too. So to prevent symbol collision with apps that use them, we'd need to
> > expose all LLVM calls we need under nome unique prefix.
> >
> > Also note that gallivm has a lot of function calls. This would also pollute
> > the app's namespace.
> >
> > In short, for this to work we should do it not for whole gallivm, but with
> > a
> > new module, just for LLVM C calls + the additional helpers, and use an
> > unique prefix (e.g., MesaLLVMXxxx) for everything.
> >
> > And we should have a new header, that #defines LLVMFoo MesaLLVMFoo, so that
> > Mesa code doesn't need to be changed all over the place.
>
> Tom sounded that the symbols are more concentrated.
> The effort is huge if you need a lot of symbols outside this library, as you
> would basically need an own abstraction of llvm on top of llvm.
What I'm suggesting doesn't require huge effort.
In detail, I'm suggesting:
(1) have a custom build of LLVM libraries with -fvisibility=hidden
(2) have a src/mesallvm/mesallvm.c containing wrappers
#include
__attribute__((visibility("default")))
LLVMValueRef
MesaLLVMBuildAdd(LLVMBuilderRef Builder, LLVMValueRef LHS, LLVMValueRef RHS,
const char *Name) {
return LLVMBuildAdd(Builder, LHS, RHS, Name);
}
...
plus a src/mesallvm/mesallvm.cpp with whatever custom C wrappers to C++
functionality we need (e.g, the stuff in lp_build_misc.cpp).
These two modules would be statically linked against the custom LLVM
libraries from (1), producing a shared library libMESALLVM.so, which would be
consumed by any Mesa driver that needs it. Because all symbols have the unique
MESALLVM prefix, these should not collide with user stuff.
(3) have a src/mesallvm/mesallvm.h
// Mange LLVM symbols before including them
#define LLVMBuildAdd MesaLLVMBuildAdd
...
#include
...
All gallium code would include src/mesallvm/mesallvm.h instead. It can still
use the LLVMFoo as befor.
And that's all there is to it.
> Is it possible to hide all llvm calls behind the lp_bld_* interface or
> similar
> and put this layer into a single shared object statically linked with all of
> llvm?
No, gallivm was not intended a full abstration to LLVM C bindings, furthermore
it is not self-contained (it depends on lots of stuff from other auxiliary
modules).
> Again OTOH, the dlopen flags are really made for this kind of module use I
> think ...
Yes, instead of what I propose above, one could have a dynamic dispatch module,
that dlopens/dlsyms everything without poluting the global namespace:
LLVMValueRef
LLVMBuildAdd((LLVMBuilderRef Builder, LLVMValueRef LHS, LLVMValueRef RHS,
const char *Name
{
LLVMBUILDADD pLLVMBuildAdd =
(LLVMBUILDADD)dlsym(dlopen("libMESALLVM.so",RTLD_LOCAL), "LLVMBuildAdd");
return pLLVMBuildAdd(Builder, LHS, RHS, Name);
}
...
It seems more work, and less efficient. I'm fine either way though.
Nevertheless, I must insist that the problem of hiding LLVM symbols is handled
in a layer underneath gallivm. Severing gallivm module from the rest auxiliary
gallium modules, or making gallivm a full wrapper for all LLVM symbols, seems
unsustainable / too limiting IMO.
Jose
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi, On Wednesday, April 24, 2013 21:54:02 Jose Fonseca wrote: > I don't see how this would work -- llvmpipe/draw has LLVMBuildXxxx calls > too. So to prevent symbol collision with apps that use them, we'd need to > expose all LLVM calls we need under nome unique prefix. > > Also note that gallivm has a lot of function calls. This would also pollute > the app's namespace. > > In short, for this to work we should do it not for whole gallivm, but with a > new module, just for LLVM C calls + the additional helpers, and use an > unique prefix (e.g., MesaLLVMXxxx) for everything. > > And we should have a new header, that #defines LLVMFoo MesaLLVMFoo, so that > Mesa code doesn't need to be changed all over the place. Tom sounded that the symbols are more concentrated. The effort is huge if you need a lot of symbols outside this library, as you would basically need an own abstraction of llvm on top of llvm. Is it possible to hide all llvm calls behind the lp_bld_* interface or similar and put this layer into a single shared object statically linked with all of llvm? Again OTOH, the dlopen flags are really made for this kind of module use I think ... Greetings Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi, On Wednesday, April 24, 2013 14:15:06 Tom Stellard wrote: > I've thought about this some more, and I think that the best solution > might be to move all LLVM API calls into gallivm and build it as a > shared object with it's own private copy of LLVM statically linked. This > way we would still have a private copy of LLVM to work with, but without > having to statically link every single gallium target with LLVM. Any > thoughts about this? That should work as well. As the test program shows, you would need to make sure that no llvm native api symbol is visible outside of this library. That means you baiscally have to build an own abstraction layer around llvm. Than you are safe for llvm at least. More notes to that below. > > For now, I have attached the patch series I mentioned regarding dlopen > > flags which could be an other approach for this. I don't mind which > > approach you really take in the end. Anyway this appears to work up to > > now. It would have the advantage that it also helps other drivers beside > > r600g with the llvm issues and it would help much further for all the non > > static private symbols we have in the drivers. The series is running here > > in my private tree since about half a year. > > A few questions about the RTL_DEEPBIND patches: > > 1. Does this only work for linux? As far as I know yes. At first even only glibc like targets. So for android I don't know for example. And I don't have one available to look into at this moment. Anyway, I think the desire to have a stronger seperation of dynamically loaded modules to be more independent of prebuilt binaries is there - especially on bsd license based userspaces. So I assume that you see more than just linux offering this or a similar flag. Sadly the times that I had access to all sorts of unices at work is gone. So for example solaris10 has RTLD_GROUP which seems to be similar. Which operating systems do we care for as of today? Is amds or intels list longer than the current list of systems for mesa? > 2. Does RTL_DEEPBIND only make a difference if we are statically linking to > LLVM? As far as I understand this should also work for a shared llvm. You should really get an own instance of all the symbols in the shared object pulled by the .so you dlopen. This should include all statics and whatnot. The limitation would be to make sure that the driver shared object finds the correct llvm libraries. In face of LD_LIBRARY_PATH or similar being set to something application defined you could still get the one provided by an application bringing its own one. This would still help for the thread safe initialization problem, but for this case you will never know which api version you are going to pull and if this might work. But, if a (closed) application ships it's own llvm libraries - which is an appealing thought because of the still fast chaning llvm libraries - you can just run into this kind of problems with about any system library that pulls llvm under the hood. So this remaining problem is nothing we can really fix in mesa. But you proposal with a libgalliumllvm abstraction helps for gallium based llvm clashes then, since the library file is called different than libllvmwhatevernativename. In general, if you care for win32 targets with such a dynamic module structure, windows dynamic linking works somehow different than the linux one. But for this problem it's most notable that the default symbol visibility is set to hidden unless stated otherwise on win32. On *nix, you can see all symbols in a shared object unless explicitly marked hidden. So by default you get symbol clashes on *nix variants. But less of them on win32. By your proposal with having a libgalliumllvm, statically linked with llvm and only exporting non llvm native symbol names you would have cought this problem already on win32. Greetings Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
- Original Message - > On Wed, Apr 24, 2013 at 09:40:44PM +0200, Mathias Fröhlich wrote: > > > > Hi Tom, > > > > On Tuesday, April 23, 2013 20:47:24 Tom Stellard wrote: > > > First of all, thanks for investigating this. The information you've > > > provided has helped me a lot. > > Good to hear that it helps. > > > > > I took a shot at implementing it this way with private static copies of > > > llvm. I've pushed the initial work to this branch: > > > git://people.freedesktop.org/~tstellar/mesa llvm-build > > > > > > I was able to hide the LLVM symbols by using the --exclude-libs linker > > > flag and so far I've tested with glxgears, an opencl example and also > > > eglgears and they all work. I still need to do some more testing, but > > > any thoughts on what I've done so far in this branch? > > > > I will look at this as soon as I find time. Probably not before the > > beginning > > of next week. > > > > I've thought about this some more, and I think that the best solution > might be to move all LLVM API calls into gallivm and build it as a > shared object with it's own private copy of LLVM statically linked. This way > we > would still have a private copy of LLVM to work with, but without having > to statically link every single gallium target with LLVM. Any thoughts > about this? I don't see how this would work -- llvmpipe/draw has LLVMBuildXxxx calls too. So to prevent symbol collision with apps that use them, we'd need to expose all LLVM calls we need under nome unique prefix. Also note that gallivm has a lot of function calls. This would also pollute the app's namespace. In short, for this to work we should do it not for whole gallivm, but with a new module, just for LLVM C calls + the additional helpers, and use an unique prefix (e.g., MesaLLVMXxxx) for everything. And we should have a new header, that #defines LLVMFoo MesaLLVMFoo, so that Mesa code doesn't need to be changed all over the place. Jose ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Wed, Apr 24, 2013 at 09:40:44PM +0200, Mathias Fröhlich wrote: > > Hi Tom, > > On Tuesday, April 23, 2013 20:47:24 Tom Stellard wrote: > > First of all, thanks for investigating this. The information you've > > provided has helped me a lot. > Good to hear that it helps. > > > I took a shot at implementing it this way with private static copies of > > llvm. I've pushed the initial work to this branch: > > git://people.freedesktop.org/~tstellar/mesa llvm-build > > > > I was able to hide the LLVM symbols by using the --exclude-libs linker > > flag and so far I've tested with glxgears, an opencl example and also > > eglgears and they all work. I still need to do some more testing, but > > any thoughts on what I've done so far in this branch? > > I will look at this as soon as I find time. Probably not before the beginning > of next week. > I've thought about this some more, and I think that the best solution might be to move all LLVM API calls into gallivm and build it as a shared object with it's own private copy of LLVM statically linked. This way we would still have a private copy of LLVM to work with, but without having to statically link every single gallium target with LLVM. Any thoughts about this? > For now, I have attached the patch series I mentioned regarding dlopen flags > which could be an other approach for this. I don't mind which approach you > really take in the end. Anyway this appears to work up to now. It would have > the advantage that it also helps other drivers beside r600g with the llvm > issues and it would help much further for all the non static private symbols > we have in the drivers. The series is running here in my private tree since > about half a year. > A few questions about the RTL_DEEPBIND patches: 1. Does this only work for linux? 2. Does RTL_DEEPBIND only make a difference if we are statically linking to LLVM? > I have also done a testcase in piglit that shows the current problem with > r600g and llvm. To be really general we should put an other symbol there > instead of the llvm context initializer. But as a first test case you can > work > with, this should be sufficient. That one really fails with current > r600g/llvm > and works with the dlopen patch series. Consequently if you want to make sure > your branch works on a private llvm version, this test needs to pass also. > This testcase is great, thanks. -Tom > Feel free to take and improove the provided patches. > > Greetings > > Mathias > From 757a111221774703d558b5807e7465a026d6a3f8 Mon Sep 17 00:00:00 2001 > Message-Id: > <757a111221774703d558b5807e7465a026d6a3f8.1366832004.git.mathias.froehl...@gmx.net> > From: Mathias Froehlich > Date: Wed, 24 Apr 2013 09:35:17 +0200 > Subject: [PATCH] Add test for driver private symbol isolation. > > --- > tests/general/CMakeLists.gl.txt | 5 > tests/general/driver-isolation-library.c | 36 ++ > tests/general/driver-isolation.c | 51 > > 3 files changed, 92 insertions(+) > create mode 100644 tests/general/driver-isolation-library.c > create mode 100644 tests/general/driver-isolation.c > > diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt > index 0e87baa..6beb745 100644 > --- a/tests/general/CMakeLists.gl.txt > +++ b/tests/general/CMakeLists.gl.txt > @@ -81,6 +81,11 @@ piglit_add_executable (line-aa-width line-aa-width.c) > IF (UNIX) > target_link_libraries (line-aa-width m) > ENDIF (UNIX) > +IF (UNIX) > + add_library (driver-isolation-library SHARED driver-isolation-library.c) > + piglit_add_executable (driver-isolation driver-isolation.c) > + target_link_libraries (driver-isolation driver-isolation-library) > +ENDIF (UNIX) > piglit_add_executable (longprim longprim.c) > piglit_add_executable (masked-clear masked-clear.c) > piglit_add_executable (pos-array pos-array.c) > diff --git a/tests/general/driver-isolation-library.c > b/tests/general/driver-isolation-library.c > new file mode 100644 > index 000..ef307a3 > --- /dev/null > +++ b/tests/general/driver-isolation-library.c > @@ -0,0 +1,36 @@ > +/* > + * Copyright (c) 2013 Mathias Fröhlich. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * on the rights to use, copy, modify, merge, publish, distribute, sub > + * license, and/or sell copies of the Software, and to permit persons to whom > + * the Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDIN
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi Tom,
On Tuesday, April 23, 2013 20:47:24 Tom Stellard wrote:
> First of all, thanks for investigating this. The information you've
> provided has helped me a lot.
Good to hear that it helps.
> I took a shot at implementing it this way with private static copies of
> llvm. I've pushed the initial work to this branch:
> git://people.freedesktop.org/~tstellar/mesa llvm-build
>
> I was able to hide the LLVM symbols by using the --exclude-libs linker
> flag and so far I've tested with glxgears, an opencl example and also
> eglgears and they all work. I still need to do some more testing, but
> any thoughts on what I've done so far in this branch?
I will look at this as soon as I find time. Probably not before the beginning
of next week.
For now, I have attached the patch series I mentioned regarding dlopen flags
which could be an other approach for this. I don't mind which approach you
really take in the end. Anyway this appears to work up to now. It would have
the advantage that it also helps other drivers beside r600g with the llvm
issues and it would help much further for all the non static private symbols
we have in the drivers. The series is running here in my private tree since
about half a year.
I have also done a testcase in piglit that shows the current problem with
r600g and llvm. To be really general we should put an other symbol there
instead of the llvm context initializer. But as a first test case you can work
with, this should be sufficient. That one really fails with current r600g/llvm
and works with the dlopen patch series. Consequently if you want to make sure
your branch works on a private llvm version, this test needs to pass also.
Feel free to take and improove the provided patches.
Greetings
Mathias
>From 757a111221774703d558b5807e7465a026d6a3f8 Mon Sep 17 00:00:00 2001
Message-Id: <757a111221774703d558b5807e7465a026d6a3f8.1366832004.git.mathias.froehl...@gmx.net>
From: Mathias Froehlich
Date: Wed, 24 Apr 2013 09:35:17 +0200
Subject: [PATCH] Add test for driver private symbol isolation.
---
tests/general/CMakeLists.gl.txt | 5
tests/general/driver-isolation-library.c | 36 ++
tests/general/driver-isolation.c | 51
3 files changed, 92 insertions(+)
create mode 100644 tests/general/driver-isolation-library.c
create mode 100644 tests/general/driver-isolation.c
diff --git a/tests/general/CMakeLists.gl.txt b/tests/general/CMakeLists.gl.txt
index 0e87baa..6beb745 100644
--- a/tests/general/CMakeLists.gl.txt
+++ b/tests/general/CMakeLists.gl.txt
@@ -81,6 +81,11 @@ piglit_add_executable (line-aa-width line-aa-width.c)
IF (UNIX)
target_link_libraries (line-aa-width m)
ENDIF (UNIX)
+IF (UNIX)
+ add_library (driver-isolation-library SHARED driver-isolation-library.c)
+ piglit_add_executable (driver-isolation driver-isolation.c)
+ target_link_libraries (driver-isolation driver-isolation-library)
+ENDIF (UNIX)
piglit_add_executable (longprim longprim.c)
piglit_add_executable (masked-clear masked-clear.c)
piglit_add_executable (pos-array pos-array.c)
diff --git a/tests/general/driver-isolation-library.c b/tests/general/driver-isolation-library.c
new file mode 100644
index 000..ef307a3
--- /dev/null
+++ b/tests/general/driver-isolation-library.c
@@ -0,0 +1,36 @@
+/*
+ * Copyright (c) 2013 Mathias Fröhlich.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * on the rights to use, copy, modify, merge, publish, distribute, sub
+ * license, and/or sell copies of the Software, and to permit persons to whom
+ * the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NON-INFRINGEMENT. IN NO EVENT SHALL VMWARE AND/OR THEIR SUPPLIERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+void
+my_important_function()
+{
+}
+
+void*
+LLVMContextCreate()
+{
+ /* If we really get here, symbol isolation does not work.
+ */
+ exit(-1);
+}
diff --git a/tests/general/driver-isolation.c b/tests/general/driver-isolation.c
new file mode 100644
index 000..ee81202
--- /dev/null
+++ b/tests/general/driver-isolation.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2013 Mathias Fröhlich.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Sat, Apr 20, 2013 at 09:27:16AM +0200, Mathias Fröhlich wrote:
>
> Hi Tom,
>
> May be I need to tell where the problem really appears in real life.
> OpenSceneGraph has some nifty features regarding multi channel rendering.
> Assume a setup of multiple full screen views running on different graphics
> boards into a single mashine composing a view into a single scene.
> Now the recommended way to do this with osg is to set up a X screen per
> graphics board. Even if this spans multiple monitors/projectors. Set up a GL
> graphics context per graphics board and set up a viewport per projector in
> the
> graphics contexts. Rendering happens now in parallel for each graphics
> context. I do drive such a thing here with two radeons and three monitors for
> testing and here the problem appears.
>
> When I start the favourite flight simulation application of my choice with
> this
> setup, then it crashes almost immediately without llvm_start_multithreaded
> being called. Wheres it works stable if we ensure llvm being multithreaded.
>
> So, I tried to distill a piglit testcase out of this somehow huger setup with
> flightgear, OpenSceneGraph, multiple gpu's and what not.
>
> On Friday, April 19, 2013 20:08:54 Tom Stellard wrote:
> > 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?
>
> Oh, no I did not see this. I did not realize that the
> llvm_start_multithreaded
> call is not just plain C. So I thought grepping for the call I used is
> sufficient.
>
> But negative. If I really apply your patch and try to run this with the above
> setup I get the crashes. The same with the piglit test here.
>
> Too bad, that reproducing races is racy for itself.
> With the piglit test I get about 2/3 of the runs either glibc memory
> corruption aborts. Or one of the below asserts from llvm:
>
> bool llvm::llvm_start_multithreaded(): Assertion `!multithreaded_mode &&
> "Already multithreaded!"' failed.
>
> void
> llvm::PassRegistry::removeRegistrationListener(llvm::PassRegistrationListener*):
>
> Assertion `I != Impl->Listeners.end() && "PassRegistrationListener not
> registered!"' failed.
>
> bool llvm::sys::SmartMutex::release() [with bool mt_only = true]:
> Assertion `((recursive && acquired) || (acquired == 1)) && "Lock not acquired
> before release!"' failed.
>
> So the biggest problem IIRC was that use of llvm::sys::SmartMutex
> which is spread around here and there in llvm. The pass registry was (is?)
> one
> of the users for that. If you did not tell llvm to run multithreaded these
> locks get noops and you concurrently access containers and that ...
>
> Looking at the first assert, the llvm guys have made this problem even worse
> IMO since I looked at this before. We need to check for multithreading being
> enabled before trying to set this. Both of which being racy for itself in
> this
> way and all of them not being save against already happening llvm access from
> an other thread and an other foreign use.
>
> > 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.
> The same here. Thanks for this.
>
> > > 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
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi all,
On Monday, April 22, 2013 00:39:57 Tom Stellard wrote:
[...]
The only pro for further investigating the dlopen flags is that I fear the
distribution builders who invented dynamic linking in the drivers. That change
destroyed symbol isolation in the drivers at that point. They will probably
argue again about the memory footprint on disc and insist on using the
distribution provided shared llvm at some point, which can only be solved then
with the dlopen approach.
Tom, to untangle your patch in question from symbol isolation, I think if you
put
static void ensure_llvm_is_multithreaded() __attribute__ ((__constructor__));
void ensure_llvm_is_multithreaded()
{
if (LLVMIsMultithreaded())
return;
LLVMStartMultithreaded();
}
into your new created c file, we should be no worse than we were before.
Regarding reproducing the problem at your site:
Does it help to increase the amount of concurrent context creations to a
higher number than the now coded 100?
> > completely agree with Mathias here. I also suggested on IRC a couple of
> > weeks ago that libllvmradeon should definitely be static and hide all
> > internal symbols and only export those needed by the drivers. That
> > should isolate us mostly from the mess that comes with having multiple
> > LLVM instances (and probably also different versions) around at the same
> > time.
>
> How would you recommend doing this? Version scripts?
Probably yes.
Greetings and Thanks
Mathias
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
On Sat, Apr 20, 2013 at 02:20:23PM +0200, Christian König wrote:
> Am 20.04.2013 09:27, schrieb Mathias Fröhlich:
> > Hi Tom,
> >
> > May be I need to tell where the problem really appears in real life.
> > OpenSceneGraph has some nifty features regarding multi channel rendering.
> > Assume a setup of multiple full screen views running on different graphics
> > boards into a single mashine composing a view into a single scene.
> > Now the recommended way to do this with osg is to set up a X screen per
> > graphics board. Even if this spans multiple monitors/projectors. Set up a GL
> > graphics context per graphics board and set up a viewport per projector in
> > the
> > graphics contexts. Rendering happens now in parallel for each graphics
> > context. I do drive such a thing here with two radeons and three monitors
> > for
> > testing and here the problem appears.
> >
> > When I start the favourite flight simulation application of my choice with
> > this
> > setup, then it crashes almost immediately without llvm_start_multithreaded
> > being called. Wheres it works stable if we ensure llvm being multithreaded.
> >
> > So, I tried to distill a piglit testcase out of this somehow huger setup
> > with
> > flightgear, OpenSceneGraph, multiple gpu's and what not.
> >
> > On Friday, April 19, 2013 20:08:54 Tom Stellard wrote:
> >> 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?
> > Oh, no I did not see this. I did not realize that the
> > llvm_start_multithreaded
> > call is not just plain C. So I thought grepping for the call I used is
> > sufficient.
> >
> > But negative. If I really apply your patch and try to run this with the
> > above
> > setup I get the crashes. The same with the piglit test here.
> >
> > Too bad, that reproducing races is racy for itself.
> > With the piglit test I get about 2/3 of the runs either glibc memory
> > corruption aborts. Or one of the below asserts from llvm:
> >
> > bool llvm::llvm_start_multithreaded(): Assertion `!multithreaded_mode &&
> > "Already multithreaded!"' failed.
> >
> > void
> > llvm::PassRegistry::removeRegistrationListener(llvm::PassRegistrationListener*):
> > Assertion `I != Impl->Listeners.end() && "PassRegistrationListener not
> > registered!"' failed.
> >
> > bool llvm::sys::SmartMutex::release() [with bool mt_only = true]:
> > Assertion `((recursive && acquired) || (acquired == 1)) && "Lock not
> > acquired
> > before release!"' failed.
> >
> > So the biggest problem IIRC was that use of llvm::sys::SmartMutex
> > which is spread around here and there in llvm. The pass registry was (is?)
> > one
> > of the users for that. If you did not tell llvm to run multithreaded these
> > locks get noops and you concurrently access containers and that ...
> >
> > Looking at the first assert, the llvm guys have made this problem even worse
> > IMO since I looked at this before. We need to check for multithreading being
> > enabled before trying to set this. Both of which being racy for itself in
> > this
> > way and all of them not being save against already happening llvm access
> > from
> > an other thread and an other foreign use.
> >
> >> 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.
> > The same here. Thanks for this.
> >
> >>> 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_mu
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Am 20.04.2013 09:27, schrieb Mathias Fröhlich:
Hi Tom,
May be I need to tell where the problem really appears in real life.
OpenSceneGraph has some nifty features regarding multi channel rendering.
Assume a setup of multiple full screen views running on different graphics
boards into a single mashine composing a view into a single scene.
Now the recommended way to do this with osg is to set up a X screen per
graphics board. Even if this spans multiple monitors/projectors. Set up a GL
graphics context per graphics board and set up a viewport per projector in the
graphics contexts. Rendering happens now in parallel for each graphics
context. I do drive such a thing here with two radeons and three monitors for
testing and here the problem appears.
When I start the favourite flight simulation application of my choice with this
setup, then it crashes almost immediately without llvm_start_multithreaded
being called. Wheres it works stable if we ensure llvm being multithreaded.
So, I tried to distill a piglit testcase out of this somehow huger setup with
flightgear, OpenSceneGraph, multiple gpu's and what not.
On Friday, April 19, 2013 20:08:54 Tom Stellard wrote:
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?
Oh, no I did not see this. I did not realize that the llvm_start_multithreaded
call is not just plain C. So I thought grepping for the call I used is
sufficient.
But negative. If I really apply your patch and try to run this with the above
setup I get the crashes. The same with the piglit test here.
Too bad, that reproducing races is racy for itself.
With the piglit test I get about 2/3 of the runs either glibc memory
corruption aborts. Or one of the below asserts from llvm:
bool llvm::llvm_start_multithreaded(): Assertion `!multithreaded_mode &&
"Already multithreaded!"' failed.
void
llvm::PassRegistry::removeRegistrationListener(llvm::PassRegistrationListener*):
Assertion `I != Impl->Listeners.end() && "PassRegistrationListener not
registered!"' failed.
bool llvm::sys::SmartMutex::release() [with bool mt_only = true]:
Assertion `((recursive && acquired) || (acquired == 1)) && "Lock not acquired
before release!"' failed.
So the biggest problem IIRC was that use of llvm::sys::SmartMutex
which is spread around here and there in llvm. The pass registry was (is?) one
of the users for that. If you did not tell llvm to run multithreaded these
locks get noops and you concurrently access containers and that ...
Looking at the first assert, the llvm guys have made this problem even worse
IMO since I looked at this before. We need to check for multithreading being
enabled before trying to set this. Both of which being racy for itself in this
way and all of them not being save against already happening llvm access from
an other thread and an other foreign use.
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.
The same here. Thanks for this.
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?
Yep, an other place would be better.
I do not know the llvm tools well enough, but If I move the current c++ code
into src/gallium/auxiliary/gallivm/lp_bld_misc.cpp it works for me (TM).
Seriously, I know of one guy who wants to use llvmpipe with windows and he
would benefit from the c++ solution s
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Hi Tom,
May be I need to tell where the problem really appears in real life.
OpenSceneGraph has some nifty features regarding multi channel rendering.
Assume a setup of multiple full screen views running on different graphics
boards into a single mashine composing a view into a single scene.
Now the recommended way to do this with osg is to set up a X screen per
graphics board. Even if this spans multiple monitors/projectors. Set up a GL
graphics context per graphics board and set up a viewport per projector in the
graphics contexts. Rendering happens now in parallel for each graphics
context. I do drive such a thing here with two radeons and three monitors for
testing and here the problem appears.
When I start the favourite flight simulation application of my choice with this
setup, then it crashes almost immediately without llvm_start_multithreaded
being called. Wheres it works stable if we ensure llvm being multithreaded.
So, I tried to distill a piglit testcase out of this somehow huger setup with
flightgear, OpenSceneGraph, multiple gpu's and what not.
On Friday, April 19, 2013 20:08:54 Tom Stellard wrote:
> 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?
Oh, no I did not see this. I did not realize that the llvm_start_multithreaded
call is not just plain C. So I thought grepping for the call I used is
sufficient.
But negative. If I really apply your patch and try to run this with the above
setup I get the crashes. The same with the piglit test here.
Too bad, that reproducing races is racy for itself.
With the piglit test I get about 2/3 of the runs either glibc memory
corruption aborts. Or one of the below asserts from llvm:
bool llvm::llvm_start_multithreaded(): Assertion `!multithreaded_mode &&
"Already multithreaded!"' failed.
void
llvm::PassRegistry::removeRegistrationListener(llvm::PassRegistrationListener*):
Assertion `I != Impl->Listeners.end() && "PassRegistrationListener not
registered!"' failed.
bool llvm::sys::SmartMutex::release() [with bool mt_only = true]:
Assertion `((recursive && acquired) || (acquired == 1)) && "Lock not acquired
before release!"' failed.
So the biggest problem IIRC was that use of llvm::sys::SmartMutex
which is spread around here and there in llvm. The pass registry was (is?) one
of the users for that. If you did not tell llvm to run multithreaded these
locks get noops and you concurrently access containers and that ...
Looking at the first assert, the llvm guys have made this problem even worse
IMO since I looked at this before. We need to check for multithreading being
enabled before trying to set this. Both of which being racy for itself in this
way and all of them not being save against already happening llvm access from
an other thread and an other foreign use.
> 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.
The same here. Thanks for this.
> > 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?
Yep, an other place would be better.
I do not know the llvm tools well enough, but If I move the current c++ code
into src/gallium/auxiliary/gallivm/lp_bld_misc.cpp it works f
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
Am 17.04.2013 01:13, schrieb Tom Stellard:
From: Tom Stellard
The LLVM C API is considered stable and should never change, so it
is much more desirable to use than the LLVM C++ API, which is constantly in
flux.
Looks good on first glance, but I would separate the initialization and
finding the correct target machine into their own functions.
Also I would guess that we need to destroy the target machine after use,
cause in the C++ version that was an autoptr.
Christian.
---
src/gallium/drivers/radeon/Makefile.am | 2 -
src/gallium/drivers/radeon/Makefile.sources | 4 +-
src/gallium/drivers/radeon/radeon_llvm_emit.c | 150 ++
src/gallium/drivers/radeon/radeon_llvm_emit.cpp | 193
src/gallium/drivers/radeon/radeon_llvm_emit.h | 14 --
5 files changed, 151 insertions(+), 212 deletions(-)
create mode 100644 src/gallium/drivers/radeon/radeon_llvm_emit.c
delete mode 100644 src/gallium/drivers/radeon/radeon_llvm_emit.cpp
diff --git a/src/gallium/drivers/radeon/Makefile.am
b/src/gallium/drivers/radeon/Makefile.am
index 6522598..9b4255e 100644
--- a/src/gallium/drivers/radeon/Makefile.am
+++ b/src/gallium/drivers/radeon/Makefile.am
@@ -27,7 +27,6 @@ endif
libllvmradeon@VERSION@_la_CXXFLAGS = \
$(GALLIUM_CFLAGS) \
- $(filter-out -DDEBUG, $(LLVM_CXXFLAGS)) \
$(DEFINES)
libllvmradeon@VERSION@_la_CFLAGS = \
@@ -35,7 +34,6 @@ libllvmradeon@VERSION@_la_CFLAGS = \
$(LLVM_CFLAGS)
libllvmradeon@VERSION@_la_SOURCES = \
- $(LLVM_CPP_FILES) \
$(LLVM_C_FILES)
libllvmradeon@VERSION@_la_LIBADD = \
diff --git a/src/gallium/drivers/radeon/Makefile.sources
b/src/gallium/drivers/radeon/Makefile.sources
index a23d5c4..d33c81b 100644
--- a/src/gallium/drivers/radeon/Makefile.sources
+++ b/src/gallium/drivers/radeon/Makefile.sources
@@ -1,9 +1,7 @@
C_SOURCES := \
radeon_uvd.c
-LLVM_CPP_FILES := \
- radeon_llvm_emit.cpp
-
LLVM_C_FILES := \
radeon_setup_tgsi_llvm.c \
+ radeon_llvm_emit.c \
radeon_llvm_util.c
diff --git a/src/gallium/drivers/radeon/radeon_llvm_emit.c
b/src/gallium/drivers/radeon/radeon_llvm_emit.c
new file mode 100644
index 000..d25b962
--- /dev/null
+++ b/src/gallium/drivers/radeon/radeon_llvm_emit.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2011 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE
+ * SOFTWARE.
+ *
+ * Authors: Tom Stellard
+ *
+ */
+#include "radeon_llvm_emit.h"
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#define CPU_STRING_LEN 30
+#define FS_STRING_LEN 30
+#define TRIPLE_STRING_LEN 7
+
+/**
+ * Set the shader type we want to compile
+ *
+ * @param type shader type to set
+ */
+void radeon_llvm_shader_type(LLVMValueRef F, unsigned type)
+{
+ char Str[2];
+ sprintf(Str, "%1d", type);
+
+ LLVMAddTargetDependentFunctionAttr(F, "ShaderType", Str);
+}
+
+/**
+ * Compile an LLVM module to machine code.
+ *
+ * @param bytes This function allocates memory for the byte stream, it is the
+ * caller's responsibility to free it.
+ */
+unsigned radeon_llvm_compile(LLVMModuleRef M, struct radeon_llvm_binary
*binary,
+ const char * gpu_family, unsigned
dump) {
+
+ static unsigned initialized = 0;
+ LLVMTargetRef target;
+ LLVMTargetMachineRef tm;
+ char cpu[CPU_STRING_LEN];
+ char fs[FS_STRING_LEN];
+ char *err;
+ LLVMMemoryBufferRef out_buffer;
+ unsigned buffer_size;
+ const char *buffer_data;
+ char triple[TRIPLE_STRING_LEN];
+ char *elf_buffer;
+ Elf *elf;
+ Elf_Scn *section = NULL;
+ size_t section_str_index;
+ int r;
+
+ if (!initialized) {
+ LLVMInitializeR600TargetInfo();
+ LLVMInitializeR600Target();
+ LLVMInitializeR600
Re: [Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
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.
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?
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.
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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] radeon/llvm: Use LLVM C API for compiling LLVM IR to ISA.
From: Tom Stellard
The LLVM C API is considered stable and should never change, so it
is much more desirable to use than the LLVM C++ API, which is constantly in
flux.
---
src/gallium/drivers/radeon/Makefile.am | 2 -
src/gallium/drivers/radeon/Makefile.sources | 4 +-
src/gallium/drivers/radeon/radeon_llvm_emit.c | 150 ++
src/gallium/drivers/radeon/radeon_llvm_emit.cpp | 193
src/gallium/drivers/radeon/radeon_llvm_emit.h | 14 --
5 files changed, 151 insertions(+), 212 deletions(-)
create mode 100644 src/gallium/drivers/radeon/radeon_llvm_emit.c
delete mode 100644 src/gallium/drivers/radeon/radeon_llvm_emit.cpp
diff --git a/src/gallium/drivers/radeon/Makefile.am
b/src/gallium/drivers/radeon/Makefile.am
index 6522598..9b4255e 100644
--- a/src/gallium/drivers/radeon/Makefile.am
+++ b/src/gallium/drivers/radeon/Makefile.am
@@ -27,7 +27,6 @@ endif
libllvmradeon@VERSION@_la_CXXFLAGS = \
$(GALLIUM_CFLAGS) \
- $(filter-out -DDEBUG, $(LLVM_CXXFLAGS)) \
$(DEFINES)
libllvmradeon@VERSION@_la_CFLAGS = \
@@ -35,7 +34,6 @@ libllvmradeon@VERSION@_la_CFLAGS = \
$(LLVM_CFLAGS)
libllvmradeon@VERSION@_la_SOURCES = \
- $(LLVM_CPP_FILES) \
$(LLVM_C_FILES)
libllvmradeon@VERSION@_la_LIBADD = \
diff --git a/src/gallium/drivers/radeon/Makefile.sources
b/src/gallium/drivers/radeon/Makefile.sources
index a23d5c4..d33c81b 100644
--- a/src/gallium/drivers/radeon/Makefile.sources
+++ b/src/gallium/drivers/radeon/Makefile.sources
@@ -1,9 +1,7 @@
C_SOURCES := \
radeon_uvd.c
-LLVM_CPP_FILES := \
- radeon_llvm_emit.cpp
-
LLVM_C_FILES := \
radeon_setup_tgsi_llvm.c \
+ radeon_llvm_emit.c \
radeon_llvm_util.c
diff --git a/src/gallium/drivers/radeon/radeon_llvm_emit.c
b/src/gallium/drivers/radeon/radeon_llvm_emit.c
new file mode 100644
index 000..d25b962
--- /dev/null
+++ b/src/gallium/drivers/radeon/radeon_llvm_emit.c
@@ -0,0 +1,150 @@
+/*
+ * Copyright 2011 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE
+ * SOFTWARE.
+ *
+ * Authors: Tom Stellard
+ *
+ */
+#include "radeon_llvm_emit.h"
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#define CPU_STRING_LEN 30
+#define FS_STRING_LEN 30
+#define TRIPLE_STRING_LEN 7
+
+/**
+ * Set the shader type we want to compile
+ *
+ * @param type shader type to set
+ */
+void radeon_llvm_shader_type(LLVMValueRef F, unsigned type)
+{
+ char Str[2];
+ sprintf(Str, "%1d", type);
+
+ LLVMAddTargetDependentFunctionAttr(F, "ShaderType", Str);
+}
+
+/**
+ * Compile an LLVM module to machine code.
+ *
+ * @param bytes This function allocates memory for the byte stream, it is the
+ * caller's responsibility to free it.
+ */
+unsigned radeon_llvm_compile(LLVMModuleRef M, struct radeon_llvm_binary
*binary,
+ const char * gpu_family, unsigned
dump) {
+
+ static unsigned initialized = 0;
+ LLVMTargetRef target;
+ LLVMTargetMachineRef tm;
+ char cpu[CPU_STRING_LEN];
+ char fs[FS_STRING_LEN];
+ char *err;
+ LLVMMemoryBufferRef out_buffer;
+ unsigned buffer_size;
+ const char *buffer_data;
+ char triple[TRIPLE_STRING_LEN];
+ char *elf_buffer;
+ Elf *elf;
+ Elf_Scn *section = NULL;
+ size_t section_str_index;
+ int r;
+
+ if (!initialized) {
+ LLVMInitializeR600TargetInfo();
+ LLVMInitializeR600Target();
+ LLVMInitializeR600TargetMC();
+ LLVMInitializeR600AsmPrinter();
+ if (!LLVMIsMultithreaded()) {
+ LLVMStartMultithreaded();
+ }
+ initialized = 1;
+ }
+
+ for (target = LLVMGetFirstTarget(); target;
+ target = LLVMGetNextTarget(targ
