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

[weiwei chen via llvm-commits]

weiweichen wrote:

> > * AArch64 (where `Ctx is MachineFunction::getContext()`)
> >   https://github.com/llvm/llvm-project/blob/3f7ca8826776f32526e948b89816db492435f2e2/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L76https://github.com/llvm/llvm-project/blob/3f7ca8826776f32526e948b89816db492435f2e2/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L118https://github.com/llvm/llvm-project/blob/3f7ca8826776f32526e948b89816db492435f2e2/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L130
> 
> Unless I'm misreadying, `Ctx` at all 3 of those locations came from a member variable of `AArch64MCInstLower`. `AArch64MCInstLower` is a member of `AArch64AsmPrinter` and the `Ctx` was initialized from `OutStreamer` not `MachineFunction::getContext()`.

Oh, good catch!! Looks like all the other backends are doing the same thing as AARch64 here that `Ctx` was initialized from `OutStreamer` not `MachineFunction::getContext()`, and X86 is the only exception.

```
(autovenv) (autovenv) weiwei.chen at Weiweis-MacBook-Pro ~/research/modularml/modular/third-party/llvm-project/llvm 🍔 git grep "MCInstLowering("
lib/Target/AArch64/AArch64AsmPrinter.cpp:99:      : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this),
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp:222:  AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp:253:  AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);
lib/Target/AMDGPU/R600MCInstLower.cpp:52:  R600MCInstLower MCInstLowering(OutContext, STI, *this);
lib/Target/ARC/ARCAsmPrinter.cpp:41:        MCInstLowering(&OutContext, *this) {}
lib/Target/AVR/AVRAsmPrinter.cpp:195:  AVRMCInstLower MCInstLowering(OutContext, *this);
lib/Target/BPF/BPFAsmPrinter.cpp:144:    BPFMCInstLower MCInstLowering(OutContext, *this);
lib/Target/CSKY/CSKYAsmPrinter.cpp:41:    : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this) {}
lib/Target/Lanai/LanaiAsmPrinter.cpp:147:  LanaiMCInstLower MCInstLowering(OutContext, *this);
lib/Target/Lanai/LanaiAsmPrinter.cpp:184:  LanaiMCInstLower MCInstLowering(OutContext, *this);
lib/Target/MSP430/MSP430AsmPrinter.cpp:149:  MSP430MCInstLower MCInstLowering(OutContext, *this);
lib/Target/Mips/MipsAsmPrinter.h:126:      : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(*this) {}
lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:695:    WebAssemblyMCInstLower MCInstLowering(OutContext, *this);
lib/Target/X86/X86MCInstLower.cpp:2204:  X86MCInstLower MCInstLowering(*MF, *this);
```

@topperc, do you by any chance know if this exception is a must to have or X86 can also switch to use OutContext for `X86MCInstLower.Ctx`? I checked that `Ctx` in X86MCInstLower` are mostly used for two things:
- `getOrCreateSymbol()` to create an MCSymbol.
- Use as an allocator to create different `MCExpr`s.
It looks like it's probably fine to switch here to be consistent with all the other backends unless there is something critical I'm missing here?

I pushed a new commit to reflect that change, [buildkite/github-pull-requests/linux-linux-x64](https://buildkite.com/llvm-project/github-pull-requests/builds/165124#0195feaa-a7ef-437e-9b61-24fd09824573) is passed. I also tested on our side that this fixes our issue as well. I'm wondering if this change is more acceptable now?



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