[PATCH] D108421: Mark openmp internal global dso_local

Tue Aug 24 04:51:28 PDT 2021

lebedev.ri added a comment.

In D108421#2962160 <https://reviews.llvm.org/D108421#2962160>, @ABataev wrote:

> In D108421#2962151 <https://reviews.llvm.org/D108421#2962151>, @lebedev.ri wrote:
>
>> In D108421#2961611 <https://reviews.llvm.org/D108421#2961611>, @kamleshbhalui wrote:
>>
>>> updated test and make changes local to auto generated global vars for lock.
>>
>> I do not understand why the most original diff here was wrong.
>> Is is the wrong default for `getOrCreateInternalVariable()` to default to `dso_local`?
>
> Need to check that the linkage/visibility of the created vars is not changing in place. If it is not changing, then it is good to go. Otherwise, need to add some another function/extra parameter with linkage/visibility  settings.

I'm having trouble parsing the comment, but i don't really see why the fact that some users may be changing linkage
should deter us from having the right default. If some users change linkage, then regardless of where this is dealt with,
the users will have to be adjusted, just a different set of them. It seems to me that fixing the ones that already mess with linkage is the right choice,
not fixing the ones that *don't* mess with linkage.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108421/new/

https://reviews.llvm.org/D108421