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

[Eli Friedman via llvm-commits]

efriedma-quic wrote:

> (sorry for being pedantic here) But to my defense, https://github.com/llvm/llvm-project/pull/108880 took 4 days to land instead of 4 hours (it was 4 hours after approval). The PR notified core maintainers like llvm/pr-subscribers-backend-x86 and requested review from folks who are not close to our org based on GitHub suggestion, but I didn't get any comments from anyone other than a post merge one about adding a test. Sorry that I probably missed/forgot that comment. Fully agree that I should have added a test to https://github.com/llvm/llvm-project/pull/108880 to indicate our intention for the change+verify the change.

I don't want to blame anyone here.  The review process for patches to LLVM relies, to a large extent, on code authors exercising good judgement, and other people catching mistakes.  And new contributors often get tripped up here.

So, the patch probably shouldn't have been merged, but it's not a big deal.  Reverts are easy enough.  And we'll make sure everyone is on the same page going forward.

------------

If we are going to make changes here, I'd like to try to ensure consistency across targets.  And that whatever API usage rule you want to impose for the sake of your out-of-tree code actually makes sense; if there's a general rule we should follow for creating MCSymbols from the AsmPrinter, we should document it and make sure we consistently follow it.

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