llvm dev - Jun 2012 - [LLVMdev] [llvm-commits] [Patch, RFC] Re: Adding support for explicitly specified TLS models (PR9788)

> OK, let's go with GeneralDynamicTLSModel then.
OK
>> The restrictions should be documented too.
>
> I'm not sure how much detail we should go into here, because the
> restrictions might vary depending on the environment. For example,
> with glibc, it will be possible to use initial-exec in a .so that will
> be loaded dynamically, given that the amount of tls memory is small
> enough, even though this doesn't work in general.
>
> I think it's better if we can have the docs say that these correspond
> to the TLS models as described in tls.pdf. I've tried to update the
> text to do that.
It is probably better to be more strict than any particular
implementation. The description on the IL definition is all the
optimizers have to play with. For example, documenting the possibility
of having a small amount of initial-exec in a dso that is dlopend
would be a bad idea, as it would prevent the compiler from lowering a
variable to initial-exec as that might go over the limit.

Something high level like:
> * localdynamic: Only used from this DSO.
> * initialexec: Will not be loaded dynamically.
> * localexec: Will be in the executable and is only used from it.
is probably OK.
>> +    default: // Map unknown non-zero value to default.
>>
>> Why?
>
> Lots of other functions in the file do this, for example
> GetDecodedLinkage and GetDecodedVisibility.
That is peculiar, but you are right, it is better be consistent.
>
>> +  return TLSModel::GeneralDynamic;
>>
>> And this return is dead, you can use llvm_unreachable.
>
> It's not dead when GV isn't a GlobalVariable. (I'm not sure
that can
> happen, though?)
>
Good point. Looks like it can also be an alias, but we were handling
that only on X86 :-( I have fixed it.

LGTM with the models documented.
>
> Thanks,
> Hans
Thanks,
Rafael

On Sat, Jun 23, 2012 at 1:35 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> It is probably better to be more strict than any particular
> implementation. The description on the IL definition is all the
> optimizers have to play with. For example, documenting the possibility
> of having a small amount of initial-exec in a dso that is dlopend
> would be a bad idea, as it would prevent the compiler from lowering a
> variable to initial-exec as that might go over the limit.
>
> Something high level like:
>
>> * localdynamic: Only used from this DSO.
>> * initialexec: Will not be loaded dynamically.
>> * localexec: Will be in the executable and is only used from it.
>
> is probably OK.
I've put that in the doc. We can tweak it more after commit if necessary.
>>> And this return is dead, you can use llvm_unreachable.
>>
>> It's not dead when GV isn't a GlobalVariable. (I'm not sure
that can
>> happen, though?)
>>
> Good point. Looks like it can also be an alias, but we were handling
> that only on X86 :-( I have fixed it.
Thanks!
> LGTM with the models documented.
Great! Committed r159077.

Thanks again for reviewing this.

 - Hans

> Great! Committed r159077.
>
> Thanks again for reviewing this.
NP, sorry for taking so long.
>  - Hans
Cheers,
Rafael