This is split for easier reading. The clang part of D125291. This revision would try to use @llvm.threadlocal.address in clang to access TLS variable. The reason why the OpenMP tests contains a lot of change is that they uses utils/update_cc_test_checks.py to update their tests.
Details
Diff Detail
Event Timeline
LG. I'm not sure this covers all places where threadlocal.address should get emitted, but I think we also don't need to get this correct right away -- for now, we we'll be able to observe if this has any negative impact on optimization, and any remaining codes will be found when we switch to a token type.
clang/lib/CodeGen/CGExpr.cpp | ||
---|---|---|
2888 | Can use withPointer here as well? |
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.
Oh, got it. I'll try to make it. And I find we can't set an intrinsic as NonNull directly in Intrinsics.td and I filed an issue for it: https://github.com/llvm/llvm-project/issues/57113.
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?
Thanks. I've filed an issue for it: https://github.com/llvm/llvm-project/issues/57438. (I'll fix it later if I had time)
Can use withPointer here as well?