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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
