[llvm] [X86Backend] Use GetExternalSymbolSymbol for MO_ExternalSymbol. (PR #133352)

[weiwei chen via llvm-commits]

================
@@ -192,8 +192,15 @@ MCSymbol *X86MCInstLower::GetSymbolFromOperand(const MachineOperand &MO) const {
   }
 
   Name += Suffix;
-  if (!Sym)
-    Sym = Ctx.getOrCreateSymbol(Name);
+  if (!Sym) {
+    // If new MCSymbol needs to be created for
+    // MachineOperand::MO_ExternalSymbol, create it as a symbol
+    // in AsmPrinter's OutContext.
+    if (MO.isSymbol())
+      Sym = AsmPrinter.OutContext.getOrCreateSymbol(Name);
+    else
+      Sym = Ctx.getOrCreateSymbol(Name);
----------------
weiweichen wrote:

> > Ctx has to be mf.getContext() as most of the codegen for this MachineFunction is in this MCContext.
> 
> I'm not really understanding what this means... I guess you've sort of informally partitioned things so that "local" symbols are created in the function's MCContext, and "global" symbols are created in the global MCContext? I don't think that's really sustainable... if we're going to consistently partition symbols, the two kinds of symbols can't both just be `MCSymbol*`, or you'll inevitably trip over issues in more obscure cases where we use the wrong MCContext.
> 

This is valid concern, but we are willing to take the risk for potentially running into issues (since we don't have substantial example right now to show this is a big problem) in the future at this point to unblock all of our tests. I'm definitely open to suggestion on how to make the MCLinker more solid (maybe after/during [my talk](https://llvm.swoogo.com/2025eurollvm/agenda) next month?).

> I don't think the patch in its current form will cause any immediate issues, but I don't want to commit to a design that hasn't really been reviewed by LLVM community, and probably won't be approved in its current form because it's very confusing.

I understand your reservation and being careful, thank you for being thorough here! Though I want to point out that this change has already been applied to most of other backends already (and I imagine those changes were being reviewed and approved by the community?). So I'd push back a bit on the assumption that this is "a design that hasn't really been reviewed by LLVM community"?



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