)]}'
{
  "commit": "1f72fa29ecb4b283f449c8bf931fcaf0fa1069ee",
  "tree": "cf096c4217829990135b16e327047d80b215794a",
  "parents": [
    "6272e1f37e0710b51d38cb98b905a3f2ffea7966"
  ],
  "author": {
    "name": "weiwei chen",
    "email": "weiwei.chen@modular.com",
    "time": "Fri Apr 04 22:44:07 2025 -0400"
  },
  "committer": {
    "name": "GitHub",
    "email": "noreply@github.com",
    "time": "Fri Apr 04 22:44:07 2025 -0400"
  },
  "message": "[X86Backend][M68KBackend] Make Ctx in X86MCInstLower (M68KInstLower) the same as AsmPrinter.OutContext  (#133352)\n\nIn `X86MCInstLower::LowerMachineOperand`, a new `MCSymbol` can be\ncreated in `GetSymbolFromOperand(MO)` where `MO.getType()` is\n`MachineOperand::MO_ExternalSymbol`\n```\n  case MachineOperand::MO_ExternalSymbol:\n    return LowerSymbolOperand(MO, GetSymbolFromOperand(MO));\n```\nat\nhttps://github.com/llvm/llvm-project/blob/725a7b664b92cd2e884806de5a08900b43d43cce/llvm/lib/Target/X86/X86MCInstLower.cpp#L196\n\nHowever, this newly created symbol will not be marked properly with its\n`IsExternal` field since `Ctx.getOrCreateSymbol(Name)` doesn\u0027t know if\nthe newly created `MCSymbol` is for `MachineOperand::MO_ExternalSymbol`.\n\n\n\nLooking at other backends, for example `Arch64MCInstLower` is doing for\nhandling `MC_ExternalSymbol`\n\n\nhttps://github.com/llvm/llvm-project/blob/14c36db16fc090ef494ff6d8207562c414b40e30/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L366-L367\n\n\nhttps://github.com/llvm/llvm-project/blob/14c36db16fc090ef494ff6d8207562c414b40e30/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp#L145-L148\n\nIt creates/gets the MCSymbol from `AsmPrinter.OutContext` instead of\nfrom `Ctx`. Moreover, `Ctx` for `AArch64MCLower` is the same as\n`AsmPrinter.OutContext`.\nhttps://github.com/llvm/llvm-project/blob/8e7d6baf0e013408be932758b4a5334c14a34086/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp#L100.\nThis applies to almost all the other backends except X86 and M68k.\n\n```\n$git grep \"MCInstLowering(\"\nlib/Target/AArch64/AArch64AsmPrinter.cpp:100:      : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this),\nlib/Target/AMDGPU/AMDGPUMCInstLower.cpp:223:  AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);\nlib/Target/AMDGPU/AMDGPUMCInstLower.cpp:257:  AMDGPUMCInstLower MCInstLowering(OutContext, STI, *this);\nlib/Target/AMDGPU/R600MCInstLower.cpp:52:  R600MCInstLower MCInstLowering(OutContext, STI, *this);\nlib/Target/ARC/ARCAsmPrinter.cpp:41:        MCInstLowering(\u0026OutContext, *this) {}\nlib/Target/AVR/AVRAsmPrinter.cpp:196:  AVRMCInstLower MCInstLowering(OutContext, *this);\nlib/Target/BPF/BPFAsmPrinter.cpp:144:    BPFMCInstLower MCInstLowering(OutContext, *this);\nlib/Target/CSKY/CSKYAsmPrinter.cpp:41:    : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(OutContext, *this) {}\nlib/Target/Lanai/LanaiAsmPrinter.cpp:147:  LanaiMCInstLower MCInstLowering(OutContext, *this);\nlib/Target/Lanai/LanaiAsmPrinter.cpp:184:  LanaiMCInstLower MCInstLowering(OutContext, *this);\nlib/Target/MSP430/MSP430AsmPrinter.cpp:149:  MSP430MCInstLower MCInstLowering(OutContext, *this);\nlib/Target/Mips/MipsAsmPrinter.h:126:      : AsmPrinter(TM, std::move(Streamer)), MCInstLowering(*this) {}\nlib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp:695:    WebAssemblyMCInstLower MCInstLowering(OutContext, *this);\nlib/Target/X86/X86MCInstLower.cpp:2200:  X86MCInstLower MCInstLowering(*MF, *this);\n```\n\nThis patch makes `X86MCInstLower` and `M68KInstLower` to have their\n`Ctx` from `AsmPrinter.OutContext` instead of getting it from\n`MF.getContext()` to be consistent with all the other backends.\n\nI think since normal use case (probably anything other than our\nun-conventional case) only handles one llvm module all the way through\nin the codegen pipeline till the end of code emission (AsmPrint),\n`AsmPrinter.OutContext` is the same as MachineFunction\u0027s MCContext, so\nthis change is an NFC.\n\n----\n\nThis fixes an error while running the generated code in ORC JIT for our\nuse case with\n[MCLinker](https://youtu.be/yuSBEXkjfEA?si\u003dHjgjkxJ9hLfnSvBj\u0026t\u003d813) (see\nmore details below):\nhttps://github.com/llvm/llvm-project/pull/133291#issuecomment-2759200983\n\nWe (Mojo) are trying to do a MC level linking so that we break llvm\nmodule into multiple submodules to compile and codegen in parallel\n(technically into *.o files with symbol linkage type change), but\ninstead of archive all of them into one `.a` file, we want to fix the\nsymbol linkage type and still produce one *.o file. The parallel codegen\npipeline generates the codegen data structures in their own `MCContext`\n(which is `Ctx` here). So if function `f` and `g` got split into\ndifferent submodules, they will have different `Ctx`. And when we try to\ncreate an external symbol with the same name for each of them with\n`Ctx.getOrCreate(SymName)`, we will get two different `MCSymbol*`\nbecause `f` and `g`\u0027s `MCContext` are different and they can\u0027t see each\nother. This is unfortunately not what we want for external symbols.\nUsing `AsmPrinter.OutContext` helps, since it is shared, if we try to\nget or create the `MCSymbol` there, we\u0027ll be able to deduplicate.",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "957c0f9d3da8271c1a3affb9834bba08a9d9e397",
      "old_mode": 33188,
      "old_path": "llvm/lib/Target/M68k/M68kMCInstLower.cpp",
      "new_id": "8698fc0de4710054ae1caad5d9ccd5d775c3b701",
      "new_mode": 33188,
      "new_path": "llvm/lib/Target/M68k/M68kMCInstLower.cpp"
    },
    {
      "type": "modify",
      "old_id": "52332c46851c2436b52183cd67962f621d7cac68",
      "old_mode": 33188,
      "old_path": "llvm/lib/Target/X86/X86MCInstLower.cpp",
      "new_id": "d9945bdf2db608be328ed1a690340a055c18897c",
      "new_mode": 33188,
      "new_path": "llvm/lib/Target/X86/X86MCInstLower.cpp"
    },
    {
      "type": "modify",
      "old_id": "4f580e7539f4d985a8bfc1a99529eecee8f972f2",
      "old_mode": 33188,
      "old_path": "llvm/unittests/CodeGen/CMakeLists.txt",
      "new_id": "a972dc32c40a215d7d6de21f24c35a3707c6fb14",
      "new_mode": 33188,
      "new_path": "llvm/unittests/CodeGen/CMakeLists.txt"
    },
    {
      "type": "add",
      "old_id": "0000000000000000000000000000000000000000",
      "old_mode": 0,
      "old_path": "/dev/null",
      "new_id": "f5a59b16b44875ac8459060309206568b740eafa",
      "new_mode": 33188,
      "new_path": "llvm/unittests/CodeGen/X86MCInstLowerTest.cpp"
    }
  ]
}