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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to