[PATCH] D110681: [X86][ISel] Lowering llvm.thread.pointer

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:26728
+    } else {
+      report_fatal_error("OS doesn't support __builtin_thread_pointer() yet.");
+    }
----------------
FreddyYe wrote:
> pengfei wrote:
> > Not sure we have to limit it to Linux here. I found other targets don't do this check.
> > Besides, we already have check in https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGBuiltin.cpp#L5040
> > If we don't support tls, we won't go here. But if we do, we should return the right thread pointer.
> Yes. X86 supports TLS in many Targets. But what I'm sure is the ELF targets has defined thread pointer on document. See the last comment here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96200
I think it makes sense to error out if we are not sure of it. Please add an error test, add fixme there and check `x86_64-apple-darwin`, `x86_64-pc-windows` etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110681