[llvm] [ThinLTO] Don't convert functions to declarations if `force-import-all` is enabled (PR #134541)

[Shilei Tian via llvm-commits]

================
@@ -1701,13 +1701,16 @@ void llvm::thinLTOFinalizeInModule(Module &TheModule,
     if (NewLinkage == GV.getLinkage())
       return;
 
+    bool ForceImportFunction = isa<Function>(GV) && ForceImportAll;
+
     // Check for a non-prevailing def that has interposable linkage
     // (e.g. non-odr weak or linkonce). In that case we can't simply
     // convert to available_externally, since it would lose the
     // interposable property and possibly get inlined. Simply drop
     // the definition in that case.
     if (GlobalValue::isAvailableExternallyLinkage(NewLinkage) &&
-        GlobalValue::isInterposableLinkage(GV.getLinkage())) {
+        GlobalValue::isInterposableLinkage(GV.getLinkage()) &&
+        !ForceImportFunction) {
----------------
shiltian wrote:

If we look strictly at whether the IR behavior conforms to the LangRef, then yes, there may be a violation. But if we take into account that object linking is not supported, the optimization you're referring to won't—and shouldn't—happen. In such cases, there would be a call edge between `main.ll` and `1.ll`, which is not supported.

The entire purpose of `force-import-all` is to work around the fact that some targets, like AMDGPU, don't support object linking. That also means cross-object connections should not exist. For AMDGPU, every module has to be self-contained. There’s no meaningful object-level linking in this context. Concepts like prevailing, non-prevailing, and interposability simply don’t apply. Linking, in this case, is effectively just a brute-force merge of object files with no actual connections among them.

I do understand your point about the LangRef violation, and I agree—it might have been a mistake to use `available_externally` linkage for all imported functions in the first place.

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