[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:

Oh.  This is starting to make more sense, now.  Normally, there's only one MCContext, but because you're doing this "parallel codegen" thing, you're passing a different MCContext to construct the MachineFunction... and the coupling between a symbol and its context is loose enough that you can pass a symbol from the wrong MCContext to the AsmPrinter, and sort of get away with it.

If preventing that is the goal, why not change X86MCInstLower::X86MCInstLower instead?  Just change `Ctx(mf.getContext())` to `Ctx(asmprinter.OutContext)`.

That said, more generally, if you want everything to work reliably, you'll probably need to completely eliminate the MCContext reference from MachineFunction.  Otherwise, you'll continue to run into issues where the MCSymbol is from the wrong context.  Maybe doable, but involves a significant amount of refactoring to avoid constructing MCSymbols early... and I'm not sure if there are any other weird interactions here.

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