[lld] [LLD][COFF] Implement support for hybrid IAT on ARM64X (PR #124189)

[Martin Storsjö via llvm-commits]

================
@@ -717,52 +717,155 @@ class ExportOrdinalChunk : public NonSectionChunk {
 void IdataContents::create(COFFLinkerContext &ctx) {
   std::vector<std::vector<DefinedImportData *>> v = binImports(ctx, imports);
 
+  // Merge compatible EC and native import files in hybrid images.
+  if (ctx.hybridSymtab) {
+    for (std::vector<DefinedImportData *> &syms : v) {
+      // At this point, symbols are sorted by base name, ensuring that
+      // compatible import files, if present, are adjacent.
+      std::vector<DefinedImportData *> hybridSyms;
+      ImportFile *prev = nullptr;
+      for (DefinedImportData *sym : syms) {
+        ImportFile *file = sym->file;
+        if (!prev || file->isEC() == prev->isEC() ||
+            !file->isSameImport(prev)) {
+          hybridSyms.push_back(sym);
+          prev = file;
+          continue;
+        }
+
+        // The native variant exposes a subset of EC symbols and chunks. Use the
+        // EC variant to represent both.
+        if (file->isEC()) {
+          hybridSyms.pop_back();
----------------
mstorsjo wrote:

Hmm, I feel unsure about this logic here, or about the assumptions it makes. Can you add more comments about how the logic of this loop works, and the assumptions? The comments currently do explain the highlevel goals of it, but I don't feel confident in the dedup logic.

If there are both native and EC imports, are they ordered so that the native one comes first - is this assumed and guaranteed?

I think what I may have a hard time wrapping my head around, is when the loop is written with the early-continue style (which may be policy and anyway generally is good structure) - the codepath we have here is essentially the special case, for `prev && file->isEC() != prev->isEC() && file->isSameImport(prev)`, i.e. deduplication. With that bit clarified for me, I have an easier time to understand the rest of the loop.

Is the check for `isEC()` required for that aspect of the deduplication? Having `isSameImport()` be true while `file->isEC() == prev->isEC()` seems unexpected. Or is it only included as a quicker way to decide this case without needing to do string comparisons?

https://github.com/llvm/llvm-project/pull/124189