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

Rafael Espíndola rafael.espindola at gmail.com
Fri Jun 22 17:35:25 PDT 2012


> 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




More information about the llvm-commits mailing list