nikic added a comment.
FWIW the bitcode patch has landed, so implementing the variant using a token
type should be possible now. I'm not sure whether it's better to start with the
current patch where the intrinsic is optional, or go directly to the one where
it is required.
================
Comment at: llvm/docs/LangRef.rst:24415
+
+ declare ptr @llvm.threadlocal.address(ptr) nounwind readnone willreturn
+
----------------
Don't we need to overload this intrinsic by return type, so it works with
different address spaces?
================
Comment at: llvm/docs/LangRef.rst:24420
+
+The first argument is pointer, which refers to a thread local variable.
+
----------------
Should we enforce here (with a verifier check) that the argument is a
thread-local global variable? I assume we //don't// want to allow weird things
like `@llvm.threadlocal.address(c ? @g1 : @g2)` right? (Though I guess, without
thread-local globals using a token type, nothing would prevent optimizations
from forming this.)
================
Comment at: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp:231
+ Changed |= lowerThreadLocalIntrinsics(F);
+ break;
}
----------------
I don't think this belongs here, this should get dropped by SelectionDAGBuilder.
================
Comment at:
llvm/test/Transforms/PreISelIntrinsicLowering/threadlocal_address.ll:1
+; RUN: opt -pre-isel-intrinsic-lowering -opaque-pointers -S -o - < %s |
FileCheck %s
+
----------------
`-opaque-pointers` flag is not necessary.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125291/new/
https://reviews.llvm.org/D125291
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
