1f72fa29ec
In `X86MCInstLower::LowerMachineOperand`, a new `MCSymbol` can be
created in `GetSymbolFromOperand(MO)` where `MO.getType()` is
`MachineOperand::MO_ExternalSymbol`
```
case MachineOperand::MO_ExternalSymbol:
return LowerSymbolOperand(MO, GetSymbolFromOperand(MO));
```
at
725a7b664b/llvm/lib/Target/X86/X86MCInstLower.cpp (L196)
However, this newly created symbol will not be marked properly with its
`IsExternal` field since `Ctx.getOrCreateSymbol(Name)` doesn't know if
the newly created `MCSymbol` is for `MachineOperand::MO_ExternalSymbol`.
Looking at other backends, for example `Arch64MCInstLower` is doing for
handling `MC_ExternalSymbol`
14c36db16f/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp (L366-L367)
14c36db16f/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp (L145-L148)
It creates/gets the MCSymbol from `AsmPrinter.OutContext` instead of
from `Ctx`. Moreover, `Ctx` for `AArch64MCLower` is the same as
`AsmPrinter.OutContext`.
8e7d6baf0e/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp (L100).
This applies to almost all the other backends except X86 and M68k.
```
$git grep "MCInstLowering("
lib/Target/AArch64/AArch64AsmPrinter.cpp💯 : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this),
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp:223: AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp:257: 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:196: 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:2200: X86MCInstLower MCInstLowering(*MF, *this);
```
This patch makes `X86MCInstLower` and `M68KInstLower` to have their
`Ctx` from `AsmPrinter.OutContext` instead of getting it from
`MF.getContext()` to be consistent with all the other backends.
I think since normal use case (probably anything other than our
un-conventional case) only handles one llvm module all the way through
in the codegen pipeline till the end of code emission (AsmPrint),
`AsmPrinter.OutContext` is the same as MachineFunction's MCContext, so
this change is an NFC.
----
This fixes an error while running the generated code in ORC JIT for our
use case with
[MCLinker](https://youtu.be/yuSBEXkjfEA?si=HjgjkxJ9hLfnSvBj&t=813) (see
more details below):
https://github.com/llvm/llvm-project/pull/133291#issuecomment-2759200983
We (Mojo) are trying to do a MC level linking so that we break llvm
module into multiple submodules to compile and codegen in parallel
(technically into *.o files with symbol linkage type change), but
instead of archive all of them into one `.a` file, we want to fix the
symbol linkage type and still produce one *.o file. The parallel codegen
pipeline generates the codegen data structures in their own `MCContext`
(which is `Ctx` here). So if function `f` and `g` got split into
different submodules, they will have different `Ctx`. And when we try to
create an external symbol with the same name for each of them with
`Ctx.getOrCreate(SymName)`, we will get two different `MCSymbol*`
because `f` and `g`'s `MCContext` are different and they can't see each
other. This is unfortunately not what we want for external symbols.
Using `AsmPrinter.OutContext` helps, since it is shared, if we try to
get or create the `MCSymbol` there, we'll be able to deduplicate.
56 lines
1.1 KiB
CMake
56 lines
1.1 KiB
CMake
set(LLVM_LINK_COMPONENTS
|
|
${LLVM_TARGETS_TO_BUILD}
|
|
Analysis
|
|
AsmParser
|
|
AsmPrinter
|
|
CodeGen
|
|
CodeGenTypes
|
|
Core
|
|
FileCheck
|
|
IRPrinter
|
|
MC
|
|
MIRParser
|
|
Passes
|
|
ScalarOpts
|
|
SelectionDAG
|
|
Support
|
|
Target
|
|
TargetParser
|
|
TransformUtils
|
|
)
|
|
|
|
add_llvm_unittest(CodeGenTests
|
|
AArch64SelectionDAGTest.cpp
|
|
AllocationOrderTest.cpp
|
|
AMDGPUMetadataTest.cpp
|
|
AsmPrinterDwarfTest.cpp
|
|
CCStateTest.cpp
|
|
DIEHashTest.cpp
|
|
DIETest.cpp
|
|
DroppedVariableStatsMIRTest.cpp
|
|
DwarfStringPoolEntryRefTest.cpp
|
|
InstrRefLDVTest.cpp
|
|
LowLevelTypeTest.cpp
|
|
LexicalScopesTest.cpp
|
|
MachineBasicBlockTest.cpp
|
|
MachineDomTreeUpdaterTest.cpp
|
|
MachineInstrBundleIteratorTest.cpp
|
|
MachineInstrTest.cpp
|
|
MachineOperandTest.cpp
|
|
RegAllocScoreTest.cpp
|
|
PassManagerTest.cpp
|
|
ScalableVectorMVTsTest.cpp
|
|
SchedBoundary.cpp
|
|
SelectionDAGAddressAnalysisTest.cpp
|
|
SelectionDAGPatternMatchTest.cpp
|
|
TypeTraitsTest.cpp
|
|
TargetOptionsTest.cpp
|
|
TestAsmPrinter.cpp
|
|
MLRegAllocDevelopmentFeatures.cpp
|
|
X86MCInstLowerTest.cpp
|
|
)
|
|
|
|
add_subdirectory(GlobalISel)
|
|
|
|
target_link_libraries(CodeGenTests PRIVATE LLVMTestingSupport)
|