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

Hans Wennborg hans at chromium.org
Thu Jun 21 02:31:07 PDT 2012


On Wed, Jun 20, 2012 at 9:29 PM, Rafael EspĂ­ndola
<rafael.espindola at gmail.com> wrote:
>> Attaching a new patch that has the behaviour we discussed.
>>
>> The "globaldynamic" and default values have been merged, and LLVM will
>> start off with the user-specified model, but choose a more specific
>> one if possible.
>>
>> Please review.
>
> Awesome, thanks!
> I will try to do a more complete review tonight or tomorrow.

Great, looking forward to landing this :)

> For now,
> just two quick observations
>
> *) This breaks the clang build, it would be nice to for the first
> commit to keep the old API. It can be removed as soon as clang is
> updated.

I have the clang patch ready (it was reviewed on cfe-commits), so my
plan was just to commit them both at once.

If we don't want to do that, I think we should try to add the new
constructors while keeping the old ones around, and then delete the
old constructors once clang is updated.

> *) Please name the most general model GeneralDynamicTLSMode. Elf's
> default visibility being called 'default' is already confusing enough
> :-)

I was thinking that calling it GeneralDynamicTLSMode wouldn't make
sense for non-ELF targets. My thinking was that DefaultTLSModel would
mean "use the default model for the target, which for ELF is general
dynamic, and on Darwin is whatever Darwin does, etc.".

> I was not sure how hard the first item would be, so I just gave it a
> try. The resulting patch is attached.

Your patch preserves the current constructors. Is the idea that we
wouldn't change the constructors, and clang would call
setThreadLocalMode() when it creates global variables? I would feel
better if we changed the constructors, to avoid the risk of forgetting
to do stuff like
NewGV->setThreadLocalMode(OldGV->getThreadLocalMode()) when new
variables are created based on old ones.

Thanks,
Hans




More information about the llvm-commits mailing list