aboutsummaryrefslogtreecommitdiff
path: root/clang/lib/Basic/SourceManager.cpp
diff options
context:
space:
mode:
authorweiwei chen <weiwei.chen@modular.com>2025-04-04 22:44:07 -0400
committerGitHub <noreply@github.com>2025-04-04 22:44:07 -0400
commit1f72fa29ecb4b283f449c8bf931fcaf0fa1069ee (patch)
treecf096c4217829990135b16e327047d80b215794a /clang/lib/Basic/SourceManager.cpp
parent6272e1f37e0710b51d38cb98b905a3f2ffea7966 (diff)
downloadllvm-1f72fa29ecb4b283f449c8bf931fcaf0fa1069ee.zip
llvm-1f72fa29ecb4b283f449c8bf931fcaf0fa1069ee.tar.gz
llvm-1f72fa29ecb4b283f449c8bf931fcaf0fa1069ee.tar.bz2
[X86Backend][M68KBackend] Make Ctx in X86MCInstLower (M68KInstLower) the same as AsmPrinter.OutContext (#133352)
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 https://github.com/llvm/llvm-project/blob/725a7b664b92cd2e884806de5a08900b43d43cce/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` https://github.com/llvm/llvm-project/blob/14c36db16fc090ef494ff6d8207562c414b40e30/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L366-L367 https://github.com/llvm/llvm-project/blob/14c36db16fc090ef494ff6d8207562c414b40e30/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`. https://github.com/llvm/llvm-project/blob/8e7d6baf0e013408be932758b4a5334c14a34086/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:100: : 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.
Diffstat (limited to 'clang/lib/Basic/SourceManager.cpp')
0 files changed, 0 insertions, 0 deletions