Skip to content

[Comgr][HotSwap] Refactor hotswap related files into their own directory#3274

Draft
chinmaydd wants to merge 3 commits into
amd-stagingfrom
amd/dev/chdeshpa/hotswap-refactor
Draft

[Comgr][HotSwap] Refactor hotswap related files into their own directory#3274
chinmaydd wants to merge 3 commits into
amd-stagingfrom
amd/dev/chdeshpa/hotswap-refactor

Conversation

@chinmaydd

Copy link
Copy Markdown

No description provided.

@chinmaydd chinmaydd requested review from harsh-amd and xintin July 8, 2026 21:28
Comment thread amd/comgr/src/hotswap/raiser/CMakeLists.txt
Comment thread amd/comgr/src/hotswap/common/code_object_utils.h
Comment thread amd/comgr/src/hotswap/raiser/CMakeLists.txt Outdated
Comment thread amd/comgr/src/hotswap/README.md
Comment thread amd/comgr/test-unit/CMakeLists.txt Outdated
@harsh-amd

harsh-amd commented Jul 8, 2026

Copy link
Copy Markdown

Just thinking out aloud here: is raiser vs rewriter the right long term abstraction here? Should we instead split it up into phases, like parse code object, discover kernels, classify source/target ISA, choose transform strategy, run transforms, update metadata/debug info, emit code object. “Rewriter” and “raiser” are execution strategies inside that pipeline, not the pipeline itself. How about something instead like:

 hotswap/
    core/              
    transforms/
      binary/
      ir/
        raise/

where
core owns the shared hotswap pieces like targets, options, code-object/kernels, diagnostics.
transforms/binary owns direct machine-code / ELF patching
transforms/ir owns raise-to-IR, LLVM lowering, and code-object re-emission

and eventually we can get a planning directory which decides what strategy to use for different kernels.

@chinmaydd chinmaydd added comgr Related to Code Object Manager hotswap Related to the Comgr Hotswap feature labels Jul 9, 2026
@chinmaydd chinmaydd force-pushed the amd/dev/chdeshpa/hotswap-refactor branch from be08c2b to 65b2339 Compare July 9, 2026 18:24
The rewriter sources previously inherited C++17 from the amd_comgr target.
As a standalone OBJECT library the standard defaulted to the compiler's
built-in level: GCC defaults to gnu++17 (so Linux built cleanly), but MSVC
defaults to C++14, which fails to compile LLVM's C++17 headers
(StringRef.h: 'data' identifier not found, basic_string ctor mismatches).
Set CMAKE_CXX_STANDARD 17 in the rewriter CMakeLists, mirroring the raiser.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chinmaydd

Copy link
Copy Markdown
Author

Thanks for engaging.

There's a couple of ways one can think about this.

The "transforms" framing makes sense if we envision that the raiser can be applied after the rewriter has made a pass-through. This may be relevant for patches or transformations that require a full-program view.

However, this is very tricky because as soon as the rewriter has run, one is assuming fixed-placement (relative or otherwise) for certain instructions. When you raise to IR, you are effectively forgoing that. The compiler may choose a different lowering or move things around. Maybe we can have a strict mode that inserts compiler barriers before and after every instruction to prevent that.

On the other hand, it may make sense to run the rewriter to run after raiser to take care of some corner cases that may not be fixable though LLVM IR. For this use case, thee raiser needs to support raising and recompiling a specific set of functions. This can be hard for indirect calls, but you can add trampolines to original function addresses to achieve that.

We need to clear up the long term vision here. My thought process aligns more towards:

hotswap/
   utils/
     <code object parsing, inst assembly, etc.>
   lib/
     rewriter/    <-- uses common utils
       patches/
     raiser/      <-- uses common utils
       passes/
   tools/
     rewriter.cpp
     raiser.cpp

Fundamentally, even though the modifications performed by the raiser and the rewriter are "transforms" I think keeping them separate and closely tied to their respective tools makes more sense to me.

The raiser is going to operate at an entirely different abstraction i.e LLVM IR whereas the rewriter only looks at machine code (ideally, I would have liked for us to atleast do this at the MCInst layer like BOLT does).

@chinmaydd chinmaydd requested review from ftynse and tgymnich July 9, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comgr Related to Code Object Manager hotswap Related to the Comgr Hotswap feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants