Rafael Espíndola
2012-Jun-12 14:41 UTC
[LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
> I thought it was a good idea to make the user's choice explicit in the > IR. If we combined the default and globaldynamic modes, LLVM wouldn't > be able to tell the difference. > > It may or may not be important to be able to tell the difference, but > it would be unfortunate if we'd have to go and change the IR format > later because we limited ourselves here. > > Also, my patch does make a difference between the default and > globaldynamic. If user specifies globaldynamic, LLVM will use that > model, even if some other model would be better (it even adds support > for doing globadynamic in non-PIC code). GCC does the same.Do you know what is the rationale for that? The static linker will optimize it anyway (but not do as good a job as the compiler could). If unsure I think it is safer to go without the 'default'. It is easier to add it to IL later than to remove it if it turns out we don't need it.> Thanks, > HansCheers, Rafael P.S.: Sorry for being so slow on the code reviews, was really busy.
Joerg Sonnenberger
2012-Jun-12 15:09 UTC
[LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
On Tue, Jun 12, 2012 at 10:41:37AM -0400, Rafael Espíndola wrote:> > I thought it was a good idea to make the user's choice explicit in the > > IR. If we combined the default and globaldynamic modes, LLVM wouldn't > > be able to tell the difference. > > > > It may or may not be important to be able to tell the difference, but > > it would be unfortunate if we'd have to go and change the IR format > > later because we limited ourselves here. > > > > Also, my patch does make a difference between the default and > > globaldynamic. If user specifies globaldynamic, LLVM will use that > > model, even if some other model would be better (it even adds support > > for doing globadynamic in non-PIC code). GCC does the same. > > Do you know what is the rationale for that? The static linker will > optimize it anyway (but not do as good a job as the compiler could).codegen can be more efficient. E.g. less or no calls to __tls_get_addr needed. Joerg
Rafael Espíndola
2012-Jun-12 16:36 UTC
[LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
>> Do you know what is the rationale for that? The static linker will >> optimize it anyway (but not do as good a job as the compiler could). > > codegen can be more efficient. E.g. less or no calls to __tls_get_addr > needed.My point also :-) What I was asking for a rationale on was *not* doing the optimization in the compiler.> JoergCheers, Rafael
Hans Wennborg
2012-Jun-12 16:39 UTC
[LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
On Tue, Jun 12, 2012 at 3:41 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:>> I thought it was a good idea to make the user's choice explicit in the >> IR. If we combined the default and globaldynamic modes, LLVM wouldn't >> be able to tell the difference. >> >> It may or may not be important to be able to tell the difference, but >> it would be unfortunate if we'd have to go and change the IR format >> later because we limited ourselves here. >> >> Also, my patch does make a difference between the default and >> globaldynamic. If user specifies globaldynamic, LLVM will use that >> model, even if some other model would be better (it even adds support >> for doing globadynamic in non-PIC code). GCC does the same. > > Do you know what is the rationale for that? The static linker will > optimize it anyway (but not do as good a job as the compiler could).I managed to dig out the original thread for GCC: http://gcc.gnu.org/ml/gcc-patches/2002-09/msg00668.html It doesn't give a rationale for the case we're discussing, though :/ My intuition still tells me that it would be good to separate the default and globaldynamic cases to 1) Respect the user's request: if the user went through the trouble of specifying __attribute__((tls_model("globaldynamic"))), we should assume there's a reason and give him what he wants, even if we think the linker is going to optimize it 2) To match GCC's behavior. I don't have any more compelling reasons than those, and I don't feel super strongly about this, so I'm willing to give in if others disagree with me :) Thanks, Hans
Rafael Espíndola
2012-Jun-12 16:45 UTC
[LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
> I managed to dig out the original thread for GCC: > http://gcc.gnu.org/ml/gcc-patches/2002-09/msg00668.html > > It doesn't give a rationale for the case we're discussing, though :/ > > My intuition still tells me that it would be good to separate the > default and globaldynamic cases to > > 1) Respect the user's request: if the user went through the trouble of > specifying __attribute__((tls_model("globaldynamic"))), we should > assume there's a reason and give him what he wants, even if we think > the linker is going to optimize it > 2) To match GCC's behavior. > > I don't have any more compelling reasons than those, and I don't feel > super strongly about this, so I'm willing to give in if others > disagree with me :)I kind of agree with the intuition, but If you don't mind I would probably ask for something stronger before changing the IL. If we do find a case where for some reason the compiler cannot optimize the model, adding a 'default' to the IL should be easy. Duncan, you know the gcc internals, any thoughts?> Thanks, > HansCheers, Rafael
Possibly Parallel Threads
- [LLVMdev] [llvm-commits] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
- [LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
- [LLVMdev] [llvm-commits] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
- [LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)
- [LLVMdev] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)