[PATCH] D129833: Use @llvm.threadlocal.address intrinsic to access TLS

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D129833#3717291 <https://reviews.llvm.org/D129833#3717291>, @jyknight wrote:

> Note that this change caused LLVM to no longer be aware that a TLS variable cannot be NULL. Thus, code like:
>
>   __thread int x;
>   int main() {
>     int* y = &x;
>     return *y;
>   }
>
> built with `clang -O -fsanitize=null` emits a null-pointer check, when it wouldn't previously. I think llvm.threadlocal.address's return value probably ought to be given the nonnull attribute. We also might want to infer other properties of the returned pointer based on knowledge of the global value parameter, such as alignment.
>
> Furthermore, while the above problem should simply be a very minor optimization degradation, in fact it caused miscompiles, due to a pre-existing bug in the X86 backend. I've sent https://reviews.llvm.org/D131716 to fix //that// part of the problem.

The NonNull attribute is added in https://github.com/llvm/llvm-project/commit/d68ba43ad24791181280fdb0f34b6be380db7a32.

And I am working on adding Align properties. But I meet problems since the alignment of threadlocal_address intrinsic depends on its argument so we can't set the alignment for its declaration and we probably need to set the alignment for its call, which means we need to set the alignment when in IRBuilder. Do you think this is good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129833