Skip to content

Conversation

@jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Nov 18, 2024

No description provided.

@jayfoad jayfoad requested a review from nikic November 18, 2024 16:40
@llvmbot llvmbot added the LTO Link time optimization (regular/full LTO or ThinLTO) label Nov 18, 2024
@jayfoad
Copy link
Contributor Author

jayfoad commented Nov 18, 2024

There is probably a lot more stuff related to type isomorphism that could be removed.

@llvmbot
Copy link
Member

llvmbot commented Nov 18, 2024

@llvm/pr-subscribers-lto

Author: Jay Foad (jayfoad)

Changes

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

1 Files Affected:

  • (modified) llvm/lib/Linker/IRMover.cpp (+2-10)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 0d54c534590ca9..c653900c632cc9 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -292,17 +292,9 @@ Type *TypeMapTy::get(Type *Ty, SmallPtrSet<StructType *, 8> &Visited) {
     AnyChange |= ElementTypes[I] != Ty->getContainedType(I);
   }
 
-  // If we found our type while recursively processing stuff, just use it.
+  // Refresh Entry after recursively processing stuff.
   Entry = &MappedTypes[Ty];
-  if (*Entry) {
-    if (auto *DTy = dyn_cast<StructType>(*Entry)) {
-      if (DTy->isOpaque()) {
-        auto *STy = cast<StructType>(Ty);
-        finishType(DTy, STy, ElementTypes);
-      }
-    }
-    return *Entry;
-  }
+  assert(!*Entry && "Recursive type!");
 
   // If all of the element types mapped directly over and the type is not
   // a named struct, then the type is usable as-is.

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jayfoad jayfoad merged commit 3d172f3 into main Nov 18, 2024
10 checks passed
@jayfoad jayfoad deleted the users/jayfoad/linker-remove-dead-recursive branch November 18, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LTO Link time optimization (regular/full LTO or ThinLTO)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants