[comgr][hotswap] Add <kernel>.stub symbols for entry trampolines#3280
[comgr][hotswap] Add <kernel>.stub symbols for entry trampolines#3280lmoriche wants to merge 2 commits into
Conversation
9fb235c to
e7d306e
Compare
lamb-j
left a comment
There was a problem hiding this comment.
LGTM, should wait for @chinmaydd and @xintin to take a look though
chinmaydd
left a comment
There was a problem hiding this comment.
LGTM, but I'd also like an idempotency test since we claim that as a feature
Idempotent: a second pass installs no new stubs and adds no duplicate symbols
Done |
| // Build the appended string names and symbol entries. | ||
| SmallVector<uint8_t> StrBlob, SymBlob; | ||
| for (const KernelEntryTrampolineFixup &F : Fixups) { | ||
| std::string Name = F.KernelName + ".stub"; |
There was a problem hiding this comment.
Should we use a complex suffix to further minimize the chance of symbol name collision?
There was a problem hiding this comment.
Compiler generated suffixes are part of the Itanium C++ ABI, and generally considered safe as '.' is not a legal character in a C++ mangled name. AFAIK, our compiler does not use ".stub", so Hotswap can use it.
|
This requires a fix due to updated API for |
lancesix
left a comment
There was a problem hiding this comment.
Just minimal comments. No concern from a debugger perspective.
With entry trampolines installed, a kernel descriptor's entry now points at the appended stub instead of the kernel body. Tools that resolve a dispatch's entry address to a name -- e.g. rocgdb `info dispatches` -- then print a bare address because the stub has no symbol. Add a `<kernel_name>.stub` STT_FUNC symbol (covering the 256-byte stub) to the code object's `.symtab` for each appended stub. rocgdb/amd-dbgapi reads the non-alloc `.symtab` and now reports the dispatch as e.g. `hello[stub]`. Only the trailing non-alloc `.symtab` / `.strtab` sections grow, so no virtual addresses, program headers, or relocations change, and `.dynsym` (used by the HSA loader) is untouched -- the rewritten object still loads and runs identically. The string blob is padded so the section header table stays 8-byte aligned. Symbol addition is best-effort: on any structural problem the rewrite keeps the (correct) symbol-less object rather than failing. Applies to every entry-trampoline rewrite. Idempotent: a second pass installs no new stubs and adds no duplicate symbols. Verified that `info dispatches` shows `hello[stub]` and the kernel runs correctly through the trampoline.
… symbols Add KernelEntryTrampoline.SecondPassAddsNoDuplicateStubSymbol (HotswapMCTest.cpp), guaranteeing the idempotency claimed by the stub-symbol change: a second rewrite pass over an already-trampolined code object installs no new stub and defines no duplicate `<kernel>.stub` symbol. The test runs the full first pass on a synthetic gfx1250 object (appendKernelEntryTrampolines -> growWithTrampolines -> rewriteKernelEntryDescriptorOffsets -> addKernelEntryTrampolineSymbols) and asserts exactly one "kernel.stub" symbol. It then re-parses that output and runs appendKernelEntryTrampolines again; since the descriptor already targets the appended stub, the second pass reports zero new stubs and empty fixups, so the symbol pass never runs. Feeding the empty fixups to addKernelEntryTrampolineSymbols returns nullptr (no new buffer), and "kernel.stub" remains defined exactly once. A helper, countSymtabSymbolsNamed, counts .symtab symbols by name.
2f6d3a3 to
124417e
Compare
With entry trampolines installed, a kernel descriptor's entry now points at the appended stub instead of the kernel body. Tools that resolve a dispatch's entry address to a name -- e.g. rocgdb
info dispatches-- then print a bare address because the stub has no symbol.Add a
<kernel_name>.stubSTT_FUNC symbol (covering the 256-byte stub) to the code object's.symtabfor each appended stub. rocgdb/amd-dbgapi reads the non-alloc.symtaband now reports the dispatch as e.g.hello[stub].Only the trailing non-alloc
.symtab/.strtabsections grow, so no virtual addresses, program headers, or relocations change, and.dynsym(used by the HSA loader) is untouched -- the rewritten object still loads and runs identically. The string blob is padded so the section header table stays 8-byte aligned. Symbol addition is best-effort: on any structural problem the rewrite keeps the (correct) symbol-less object rather than failing.Applies to every entry-trampoline rewrite. Idempotent: a second pass installs no new stubs and adds no duplicate symbols.
Verified that
info dispatchesshowshello[stub]and the kernel runs correctly through the trampoline.