[PATCH] D79005: [LLD][COFF] Move debug info for thread-local variables into PDB global stream

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.

lgtm with a comment update, thanks!



================
Comment at: lld/COFF/PDB.cpp:808
   case SymbolKind::S_UDT:
     return !isGlobalScope;
   // S_GDATA32 does not go in the module stream, but S_LDATA32 does.
----------------
aganea wrote:
> rnk wrote:
> > Looks like we need to send S_LDATA32 to here.
> Not exactly. `S_LDATA32` always remains in the module stream, but is duplicated into the globals stream only if it's a global.
Ah, yes, this comment should've been in `symbolGoesInGlobalsStream`.


================
Comment at: lld/COFF/PDB.cpp:830
     return true;
-  // S_UDT records go in the globals stream if it is a global S_UDT.
+  // Records that go in the globals stream, if they represent a global variable.
   case SymbolKind::S_UDT:
----------------
UDTs aren't global variables. I'd change the qualification to "unless they are function-local". At the end of the day, the linker only knows if the record is inside a matched pair of `S_[LG]PROC32` / `S_END` records, i.e. function local. Maybe `isGlobalScope` should be renamed to `isFunctionScope` and inverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79005