[LLD][COFF][NFC] Store live flag in ImportThunkChunk.

[cjacek]

Instead of ImportFile. This is a preparation for ARM64EC support, which has both x86 and ARM64EC thunks and each of them needs a separate flag.

[llvmbot]

@llvm/pr-subscribers-lld
@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

Instead of ImportFile. This is a preparation for ARM64EC support, which has both x86 and ARM64EC thunks and each of them needs a separate flag.

---

Full diff: https://github.com/llvm/llvm-project/pull/108459.diff

10 Files Affected:

• (modified) lld/COFF/Chunks.cpp (+4)
• (modified) lld/COFF/Chunks.h (+5-2)
• (modified) lld/COFF/InputFiles.cpp (+1-1)
• (modified) lld/COFF/InputFiles.h (+1-4)
• (modified) lld/COFF/MapFile.cpp (+1-1)
• (modified) lld/COFF/MarkLive.cpp (+1-1)
• (modified) lld/COFF/PDB.cpp (+2-2)
• (modified) lld/COFF/Symbols.cpp (+1-1)
• (modified) lld/COFF/Symbols.h (+2-2)
• (modified) lld/COFF/Writer.cpp (+1-1)

diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 0f33885f7df37d..ee54fa39fc3d6a 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -774,6 +774,10 @@ void StringChunk::writeTo(uint8_t *buf) const {
   buf[str.size()] = '\0';
 }
 
+ImportThunkChunk::ImportThunkChunk(COFFLinkerContext &ctx, Defined *s)
+    : NonSectionCodeChunk(ImportThunkKind), live(!ctx.config.doGC),
+      impSymbol(s), ctx(ctx) {}
+
 ImportThunkChunkX64::ImportThunkChunkX64(COFFLinkerContext &ctx, Defined *s)
     : ImportThunkChunk(ctx, s) {
   // Intel Optimization Manual says that all branch targets
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
index 040a249aabf59e..8ad17a28507822 100644
--- a/lld/COFF/Chunks.h
+++ b/lld/COFF/Chunks.h
@@ -557,10 +557,13 @@ static const uint8_t importThunkARM64EC[] = {
 // contents will be a JMP instruction to some __imp_ symbol.
 class ImportThunkChunk : public NonSectionCodeChunk {
 public:
-  ImportThunkChunk(COFFLinkerContext &ctx, Defined *s)
-      : NonSectionCodeChunk(ImportThunkKind), impSymbol(s), ctx(ctx) {}
+  ImportThunkChunk(COFFLinkerContext &ctx, Defined *s);
   static bool classof(const Chunk *c) { return c->kind() == ImportThunkKind; }
 
+  // We track the usage of the thunk symbol separately from the import file
+  // to avoid generating unnecessary thunks.
+  bool live;
+
 protected:
   Defined *impSymbol;
   COFFLinkerContext &ctx;
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index 569220468e96ad..ee39b466244448 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -1002,7 +1002,7 @@ void ObjFile::enqueuePdbFile(StringRef path, ObjFile *fromFile) {
 }
 
 ImportFile::ImportFile(COFFLinkerContext &ctx, MemoryBufferRef m)
-    : InputFile(ctx, ImportKind, m), live(!ctx.config.doGC), thunkLive(live) {}
+    : InputFile(ctx, ImportKind, m), live(!ctx.config.doGC) {}
 
 MachineTypes ImportFile::getMachineType() const {
   uint16_t machine =
diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h
index 8140a031f71166..0812e9c4610457 100644
--- a/lld/COFF/InputFiles.h
+++ b/lld/COFF/InputFiles.h
@@ -371,11 +371,8 @@ class ImportFile : public InputFile {
   // are actually in use.
   //
   // If the Live bit is turned off by MarkLive, Writer will ignore dllimported
-  // symbols provided by this import library member. We also track whether the
-  // imported symbol is used separately from whether the thunk is used in order
-  // to avoid creating unnecessary thunks.
+  // symbols provided by this import library member.
   bool live;
-  bool thunkLive;
 };
 
 // Used for LTO.
diff --git a/lld/COFF/MapFile.cpp b/lld/COFF/MapFile.cpp
index ed521dd375ed01..52e9ce996f2390 100644
--- a/lld/COFF/MapFile.cpp
+++ b/lld/COFF/MapFile.cpp
@@ -125,7 +125,7 @@ static void getSymbols(const COFFLinkerContext &ctx,
     if (!file->thunkSym)
       continue;
 
-    if (!file->thunkLive)
+    if (!file->thunkSym->isLive())
       continue;
 
     if (auto *thunkSym = dyn_cast<Defined>(file->thunkSym))
diff --git a/lld/COFF/MarkLive.cpp b/lld/COFF/MarkLive.cpp
index 8af58780e13582..3c09baa73a9f7b 100644
--- a/lld/COFF/MarkLive.cpp
+++ b/lld/COFF/MarkLive.cpp
@@ -58,7 +58,7 @@ void markLive(COFFLinkerContext &ctx) {
       addImportFile(sym->file);
     } else if (auto *sym = dyn_cast<DefinedImportThunk>(b)) {
       addImportFile(sym->wrappedSym->file);
-      sym->wrappedSym->file->thunkLive = true;
+      sym->getChunk()->live = true;
     }
   };
 
diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp
index c0739b37aeb0f3..9b035f53ef49cf 100644
--- a/lld/COFF/PDB.cpp
+++ b/lld/COFF/PDB.cpp
@@ -1527,8 +1527,8 @@ void PDBLinker::addImportFilesToPDB() {
     if (!file->thunkSym)
       continue;
 
-    if (!file->thunkLive)
-        continue;
+    if (!file->thunkSym->isLive())
+      continue;
 
     std::string dll = StringRef(file->dllName).lower();
     llvm::pdb::DbiModuleDescriptorBuilder *&mod = dllToModuleDbi[dll];
diff --git a/lld/COFF/Symbols.cpp b/lld/COFF/Symbols.cpp
index 5f4d797f74a2dd..567c2b93776c94 100644
--- a/lld/COFF/Symbols.cpp
+++ b/lld/COFF/Symbols.cpp
@@ -84,7 +84,7 @@ bool Symbol::isLive() const {
   if (auto *imp = dyn_cast<DefinedImportData>(this))
     return imp->file->live;
   if (auto *imp = dyn_cast<DefinedImportThunk>(this))
-    return imp->wrappedSym->file->thunkLive;
+    return imp->getChunk()->live;
   // Assume any other kind of symbol is live.
   return true;
 }
diff --git a/lld/COFF/Symbols.h b/lld/COFF/Symbols.h
index 2df60a01ec813d..9b21e09bf83a42 100644
--- a/lld/COFF/Symbols.h
+++ b/lld/COFF/Symbols.h
@@ -395,12 +395,12 @@ class DefinedImportThunk : public Defined {
   }
 
   uint64_t getRVA() { return data->getRVA(); }
-  Chunk *getChunk() { return data; }
+  ImportThunkChunk *getChunk() const { return data; }
 
   DefinedImportData *wrappedSym;
 
 private:
-  Chunk *data;
+  ImportThunkChunk *data;
 };
 
 // If you have a symbol "foo" in your object file, a symbol name
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
index 9a8040008e73ca..0b3c4163020f45 100644
--- a/lld/COFF/Writer.cpp
+++ b/lld/COFF/Writer.cpp
@@ -1258,7 +1258,7 @@ void Writer::appendImportThunks() {
     if (!isa<DefinedImportThunk>(file->thunkSym))
       fatal(toString(ctx, *file->thunkSym) + " was replaced");
     DefinedImportThunk *thunk = cast<DefinedImportThunk>(file->thunkSym);
-    if (file->thunkLive)
+    if (thunk->getChunk()->live)
       textSec->addChunk(thunk->getChunk());
     if (file->impchkThunk)
       textSec->addChunk(file->impchkThunk);

[mstorsjo]

LGTM

[mstorsjo]

I presume the move from header to cpp file is because you want to access ctx.config here, which otherwise isn't visible?

[cjacek]

Yes, that was the reason.

[mstorsjo]

This bit had me a bit surprised here, when reading the context of the code below: Do I understand the existing logic here, that if we only access __imp_foo but not foo, we don't emit either of them into the map file, but if we access foo, we emit both into the map file?

(Although if GC isn't enabled, then all chunks are considered live, and they're emitted into the map file anyway.)

[cjacek]

I believe that in most cases, these symbols will be emitted regardless of the loop. They are typically referenced by some object file and pulled in earlier during the function execution. If no object file references the symbol, it wouldn’t be pulled in the first place. However, if the code only references the thunk foo, this implicitly requires __imp_foo to be defined as well. I assume this pull is intended to handle that specific case.

In the context of ARM64EC, there are additional symbols implicitly defined, so this logic won’t fully cover those scenarios. I'll take a closer look at that.