Reland "[ORC] Track __emutls_t definitions in IRMaterializationUnit" (#207161)

[conrade-ctc]

Relands #207161 (the IRMaterializationUnit::discard fix for duplicated
__emutls_t.<var> symbols). History: #207161 merged, reverted in #207775,
relanded in #208413, reverted again in #209260 (a day before the 23.x cut).

Why #208413 was reverted

clang-repl's JIT lowers thread_local to emulated TLS on every target
(JITTargetMachineBuilder forces EmulatedTLS), so JIT'd code always
references __emutls_get_address. #208413 only made that symbol resolvable on
Darwin (#ifdef __APPLE__ + isOSBinFormatMachO()), where it lives in the
compiler-rt builtins static archive. The same gap exists on any host that
links compiler-rt builtins instead of libgcc_s: the arm-toolchain CI hit it on
Linux/AArch64 built with -rtlib=compiler-rt (no libgcc_s), where
__emutls_get_address is only in the static archive and process-symbol lookup
cannot find it — JIT session error: Symbols not found: [ __emutls_get_address ].
(glibc bots pass because libgcc_s.so exports it.)

This reland

The IRMaterializationUnit fix (Layer.cpp) and the test are unchanged from
#208413. The IncrementalExecutor.cpp workaround is broadened from Darwin-only
to all in-process Unix hosts:

• reference __emutls_get_address under LLVM_ON_UNIX (excluding Emscripten,
  AIX, and z/OS) to force-link it from whichever compiler runtime the toolchain
  uses (libgcc_s or the compiler-rt builtins archive), and
• define it as an absolute symbol in the process-symbols JITDylib regardless of
  target binary format.

This is target-independent: emulated-TLS lowering emits the accessor name from
shared codegen (TargetLowering), so a single symbol covers every arch. The
previous isOSBinFormatMachO() guard is dropped — with emulated TLS forced on,
a target-format check is vestigial and can only re-exclude the exact hosts that
fail. Where process-symbol lookup already resolves the symbol (glibc), the
absolute definition is a harmless shadow (an already-defined symbol is dropped
from generation candidates before the process-symbol generator runs), following
the MinGW __main precedent in lli.cpp. In-process only — a host address is
meaningless for an out-of-process executor. The UNSUPPORTED: target=loongarch
line from #208886 is retained (LoongArch rejects emulated TLS outright).

The reference resolves from the toolchain's compiler runtime; hosts with no
guaranteed emulated-TLS runtime on the link line (MSVC, Emscripten, AIX, z/OS)
are excluded and fall back to process-symbol lookup, as before.

Alternatives considered

• Load the compiler-rt builtins archive into a JITDylib (via a
  static-library definition generator, as lli --extra-archive does) instead of
  force-linking + defining the symbol. This avoids the build-time link
  dependency and generalizes to other builtins, at the cost of runtime discovery
  of the builtins archive path (resource dir / sysroot / cross-compiles /
  libgcc-only toolchains) plus plumbing to reach the in-process JIT path. It's a
  reasonable alternative and a clean follow-on; this PR takes the smaller,
  target-independent route to get the discard fix relanded, and I'm happy to
  switch to it if reviewers prefer.
• Native TLS + the ORC runtime — the long-term fix (also removes the
  in-process-only constraint), tracked by the TODO added next to the workaround.
  Out of scope here.

Testing

Verified on Linux/x86-64: full check-clang clean; clang/test/Interpreter/emulated-tls.cpp
passes. Darwin verified by the author on aarch64.

🤖 Done with the help of Claude Code (Claude Opus 4.8, human in the loop)

[conrade-ctc]

@vgvassilev @lhames — this is the reland of #208413 (reverted in #209260 just before the 23.x cut). The Darwin-only symbol fix left the same __emutls_get_address gap on compiler-rt-rtlib ELF hosts (the arm-toolchain CI caught it on Linux/AArch64), so this broadens the workaround to all in-process Unix hosts and drops the isOSBinFormatMachO() guard — with emulated TLS forced on, a target-format check only re-excludes the hosts that fail. There's an "Alternatives considered" note in the description on loading the builtins archive into a JITDylib instead; glad to go that route if you'd prefer it. Would appreciate a look when you have a moment.

[vgvassilev]

Can we ask the bot owners who found the regression to review?

[conrade-ctc]

Can we ask the bot owners who found the regression to review?

Makes sense, @pawosm-arm, @vrukesh, this should pass through your CI now if you'd like to review

[pawosm-arm]

Can we ask the bot owners who found the regression to review?

Makes sense, @pawosm-arm, @vrukesh, this should pass through your CI now if you'd like to review

I doubt this CI is capable of verifying an arbitrary upstream commit: things need to be pulled in from the main branch before they could be ground by it. I'd suggest you to get it reviewed, merge it, and then we'll see if it works.

[vgvassilev]

Looks reasonable to me but I’d like to wait for @lhames.

[conrade-ctc]

@lhames gentle ping — this reland has been sitting green for a couple of weeks; would appreciate a look when you get a chance. Thanks!

[conrade-ctc]

@vgvassilev, @lhames, more than happy to massage this one if there is something that needs to be addressed, anything here look off to you guys?

[vgvassilev]

Let's move forward and rely on post merge feedback.

[brooksdavis]

This broke the build on FreeBSD where __emutls_get_address is not provided. This seems sufficient to fix the build:

--- clang/lib/Interpreter/IncrementalExecutor.cpp.orig
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -76,7 +76,7 @@
 // and AIX / z/OS (whose runtimes may not provide the symbol). On those hosts
 // thread_locals instead rely on process-symbol lookup, unchanged from before.
 #if defined(LLVM_ON_UNIX) && !defined(__EMSCRIPTEN__) && !defined(_AIX) &&     \
-    !defined(__MVS__)
+    !defined(__MVS__) && !defined(__FreeBSD__)
 extern "C" void *__emutls_get_address(void *);
 static void *getEmuTLSGetAddressPtr() {
   return reinterpret_cast<void *>(&__emutls_get_address);

Whatever tool did the line wrapping of the commit message is appalling. It's basically unreadable. :(

[vgvassilev]

This broke the build on FreeBSD where __emutls_get_address is not provided. This seems sufficient to fix the build:

--- clang/lib/Interpreter/IncrementalExecutor.cpp.orig
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -76,7 +76,7 @@
 // and AIX / z/OS (whose runtimes may not provide the symbol). On those hosts
 // thread_locals instead rely on process-symbol lookup, unchanged from before.
 #if defined(LLVM_ON_UNIX) && !defined(__EMSCRIPTEN__) && !defined(_AIX) &&     \
-    !defined(__MVS__)
+    !defined(__MVS__) && !defined(__FreeBSD__)
 extern "C" void *__emutls_get_address(void *);
 static void *getEmuTLSGetAddressPtr() {
   return reinterpret_cast<void *>(&__emutls_get_address);

Whatever tool did the line wrapping of the commit message is appalling. It's basically unreadable. :(

Sorry about that. I noticed the formatting but I thought it was only on my end. Do you mind committing that fix of yours?

[brooksdavis]

Sorry about that. I noticed the formatting but I thought it was only on my end. Do you mind committing that fix of yours?

I don't have commit permissions, but I've created #217004

[vgvassilev]

Approved but let’s wait for the pre-merge checks!

[conrade-ctc]

--- clang/lib/Interpreter/IncrementalExecutor.cpp.orig
+++ clang/lib/Interpreter/IncrementalExecutor.cpp
@@ -76,7 +76,7 @@
// and AIX / z/OS (whose runtimes may not provide the symbol). On those hosts
// thread_locals instead rely on process-symbol lookup, unchanged from before.
#if defined(LLVM_ON_UNIX) && !defined(EMSCRIPTEN) && !defined(_AIX) && \

• !defined(MVS)

• !defined(MVS) && !defined(FreeBSD)
  extern "C" void *__emutls_get_address(void *);
  static void *getEmuTLSGetAddressPtr() {
  return reinterpret_cast<void *>(&__emutls_get_address);

@brooksdavis, I just double-checked and it was a pass of clang-format --style=LLVM that is responsible for formatting the line break that way! Probably needs a tweak to the ColumnLimit: 80, AlignEscapedNewlines: Right, ContinuationIndentWidth: 4 that causes it... agree it's ugly :(

[pranavk]

this is also breaking us in Google. We are LLVM_ON_UNIX and we don't have __emutls_get_address in our runtime.

[pranavk]

My colleague @rupprecht suggested cmake variable to guard this with. Could we have some HAS_EMUTLS or similar for this instead of current approach which forces downstream compilers to provide definition of emutls_get_address?

[vgvassilev]

My colleague @rupprecht suggested cmake variable to guard this with. Could we have some HAS_EMUTLS or similar for this instead of current approach which forces downstream compilers to provide definition of emutls_get_address?

That seems to be a good way forward. Do we have a PR with it upstream?