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

[Eli Friedman 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);
----------------
efriedma-quic 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.

-----

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.

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