Skip to content

Commit efa8111

Browse files
harsh-amdharsh-amd
committed
AMDGPU: reduce hotswap helper duplication
1 parent 292d25e commit efa8111

4 files changed

Lines changed: 74 additions & 81 deletions

File tree

‎amd/comgr/src/comgr-hotswap-b0a0.cpp‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,22 @@ static void runScratchVerification(WritableMemoryBuffer &OutBuf,
522522
<< "scratch conflicts\n";
523523
}
524524

525+
static std::unique_ptr<WritableMemoryBuffer>
526+
copyOutputBuffer(const void *Data, size_t Size, StringRef CopyKind) {
527+
std::unique_ptr<WritableMemoryBuffer> Result =
528+
WritableMemoryBuffer::getNewUninitMemBuffer(Size);
529+
if (!Result) {
530+
log() << "hotswap: error: retargetCodeObject: "
531+
<< "getNewUninitMemBuffer(" << Size
532+
<< ") failed (out of memory) for the " << CopyKind
533+
<< " output copy.\n";
534+
return nullptr;
535+
}
536+
537+
std::memcpy(Result->getBufferStart(), Data, Size);
538+
return Result;
539+
}
540+
525541
// -- retargetCodeObject -------------------------------------------------------
526542

527543
amd_comgr_status_t retargetCodeObject(const void *ElfData, size_t ElfSize,
@@ -535,14 +551,9 @@ amd_comgr_status_t retargetCodeObject(const void *ElfData, size_t ElfSize,
535551

536552
if (!Options.RunB0A0Patches && !Options.RunEntryTrampolines) {
537553
std::unique_ptr<WritableMemoryBuffer> Result =
538-
WritableMemoryBuffer::getNewUninitMemBuffer(ElfSize);
539-
if (!Result) {
540-
log() << "hotswap: error: retargetCodeObject: "
541-
<< "getNewUninitMemBuffer(" << ElfSize
542-
<< ") failed (out of memory) for the no-op output copy.\n";
554+
copyOutputBuffer(ElfData, ElfSize, "no-op");
555+
if (!Result)
543556
return AMD_COMGR_STATUS_ERROR_OUT_OF_RESOURCES;
544-
}
545-
std::memcpy(Result->getBufferStart(), ElfData, ElfSize);
546557
Out = std::move(Result);
547558
return AMD_COMGR_STATUS_SUCCESS;
548559
}
@@ -638,14 +649,9 @@ amd_comgr_status_t retargetCodeObject(const void *ElfData, size_t ElfSize,
638649
EntryFixups))
639650
return AMD_COMGR_STATUS_ERROR;
640651
} else {
641-
Result = WritableMemoryBuffer::getNewUninitMemBuffer(ElfSize);
642-
if (!Result) {
643-
log() << "hotswap: error: retargetCodeObject: "
644-
<< "getNewUninitMemBuffer(" << ElfSize
645-
<< ") failed (out of memory) for the patched output copy.\n";
652+
Result = copyOutputBuffer(Buf.data(), ElfSize, "patched");
653+
if (!Result)
646654
return AMD_COMGR_STATUS_ERROR_OUT_OF_RESOURCES;
647-
}
648-
std::memcpy(Result->getBufferStart(), Buf.data(), ElfSize);
649655
}
650656

651657
if (!ScratchPatches.empty())

‎amd/comgr/src/comgr-hotswap-elf.cpp‎

Lines changed: 20 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
#include "llvm/ADT/StringExtras.h"
1919
#include "llvm/ADT/Twine.h"
2020
#include "llvm/BinaryFormat/MsgPackDocument.h"
21-
#include "llvm/Support/CheckedArithmetic.h"
2221

2322
#include <algorithm>
2423
#include <limits>
@@ -42,16 +41,6 @@ enum class MetadataSgprUpdateStatus {
4241
Error,
4342
};
4443

45-
static std::optional<uint64_t> checkedAdd(uint64_t LHS, uint64_t RHS,
46-
StringRef Context) {
47-
std::optional<uint64_t> Result = checkedAddUnsigned(LHS, RHS);
48-
if (Result)
49-
return Result;
50-
51-
log() << "hotswap: error: " << Context << " overflows uint64_t.\n";
52-
return std::nullopt;
53-
}
54-
5544
static std::optional<size_t>
5645
checkedAlignToSize(size_t Value, uint64_t Alignment, StringRef Context) {
5746
if (Alignment <= 1)
@@ -61,7 +50,7 @@ checkedAlignToSize(size_t Value, uint64_t Alignment, StringRef Context) {
6150
if (Remainder == 0)
6251
return Value;
6352
std::optional<uint64_t> Aligned =
64-
checkedAdd(Value64, Alignment - Remainder, Context);
53+
checkedAddUint64(Value64, Alignment - Remainder, Context);
6554
if (!Aligned)
6655
return std::nullopt;
6756
if (*Aligned > static_cast<uint64_t>(std::numeric_limits<size_t>::max())) {
@@ -84,8 +73,8 @@ static std::optional<uint64_t> checkedSectionFileOffset(const ELFT::Shdr &Sec,
8473
}
8574

8675
uint64_t Delta = VAddr - Sec.sh_addr;
87-
std::optional<uint64_t> FileOffset =
88-
checkedAdd(Sec.sh_offset, Delta, (Twine(Context) + " file offset").str());
76+
std::optional<uint64_t> FileOffset = checkedAddUint64(
77+
Sec.sh_offset, Delta, (Twine(Context) + " file offset").str());
8978
if (!FileOffset)
9079
return std::nullopt;
9180

@@ -837,7 +826,7 @@ static bool adjustSectionHeaders(uint8_t *Elf, size_t ElfSize,
837826
return true;
838827

839828
std::optional<uint64_t> TextEnd =
840-
checkedAdd(TextOffset, TextSize, "section header .text end");
829+
checkedAddUint64(TextOffset, TextSize, "section header .text end");
841830
if (!TextEnd)
842831
return false;
843832
uint64_t Shoff;
@@ -851,7 +840,7 @@ static bool adjustSectionHeaders(uint8_t *Elf, size_t ElfSize,
851840

852841
if (Shoff >= *TextEnd) {
853842
std::optional<uint64_t> NewShoff =
854-
checkedAdd(Shoff, TrampTotal, "section header table offset");
843+
checkedAddUint64(Shoff, TrampTotal, "section header table offset");
855844
if (!NewShoff)
856845
return false;
857846
uint64_t NewShoffValue = *NewShoff;
@@ -863,7 +852,7 @@ static bool adjustSectionHeaders(uint8_t *Elf, size_t ElfSize,
863852
for (uint16_t I = 0; I < Shnum; ++I) {
864853
uint64_t ShTableDelta = static_cast<uint64_t>(I) * Shentsize;
865854
std::optional<uint64_t> ShPos =
866-
checkedAdd(Shoff, ShTableDelta, "section header entry offset");
855+
checkedAddUint64(Shoff, ShTableDelta, "section header entry offset");
867856
if (!ShPos)
868857
return false;
869858
if (*ShPos > ElfSize || sizeof(Shdr) > ElfSize - *ShPos)
@@ -874,15 +863,15 @@ static bool adjustSectionHeaders(uint8_t *Elf, size_t ElfSize,
874863

875864
if (ShOffset == TextOffset) {
876865
std::optional<uint64_t> NewTextSize =
877-
checkedAdd(TextSize, TrampTotal, ".text section size");
866+
checkedAddUint64(TextSize, TrampTotal, ".text section size");
878867
if (!NewTextSize)
879868
return false;
880869
uint64_t NewTextSizeValue = *NewTextSize;
881870
std::memcpy(Sh + offsetof(Shdr, sh_size), &NewTextSizeValue,
882871
sizeof(NewTextSizeValue));
883872
} else if (ShOffset > TextOffset) {
884873
std::optional<uint64_t> NewOffset =
885-
checkedAdd(ShOffset, TrampTotal, "post-.text section offset");
874+
checkedAddUint64(ShOffset, TrampTotal, "post-.text section offset");
886875
if (!NewOffset)
887876
return false;
888877
uint64_t NewOffsetValue = *NewOffset;
@@ -894,7 +883,7 @@ static bool adjustSectionHeaders(uint8_t *Elf, size_t ElfSize,
894883
uint64_t ShAddr;
895884
std::memcpy(&ShAddr, Sh + offsetof(Shdr, sh_addr), sizeof(ShAddr));
896885
std::optional<uint64_t> NewAddr =
897-
checkedAdd(ShAddr, TrampTotal, "post-.text section address");
886+
checkedAddUint64(ShAddr, TrampTotal, "post-.text section address");
898887
if (!NewAddr)
899888
return false;
900889
ShAddr = *NewAddr;
@@ -912,7 +901,7 @@ static bool adjustProgramHeaders(uint8_t *Elf, size_t ElfSize,
912901
return true;
913902

914903
std::optional<uint64_t> TextEnd =
915-
checkedAdd(TextOffset, TextSize, "program header .text end");
904+
checkedAddUint64(TextOffset, TextSize, "program header .text end");
916905
if (!TextEnd)
917906
return false;
918907
uint64_t Phoff;
@@ -927,7 +916,7 @@ static bool adjustProgramHeaders(uint8_t *Elf, size_t ElfSize,
927916
for (uint16_t I = 0; I < Phnum; ++I) {
928917
uint64_t PhTableDelta = static_cast<uint64_t>(I) * Phentsize;
929918
std::optional<uint64_t> PhPos =
930-
checkedAdd(Phoff, PhTableDelta, "program header entry offset");
919+
checkedAddUint64(Phoff, PhTableDelta, "program header entry offset");
931920
if (!PhPos)
932921
return false;
933922
if (*PhPos > ElfSize || sizeof(Phdr) > ElfSize - *PhPos)
@@ -941,14 +930,14 @@ static bool adjustProgramHeaders(uint8_t *Elf, size_t ElfSize,
941930
std::memcpy(&PMemsz, Ph + offsetof(Phdr, p_memsz), sizeof(PMemsz));
942931

943932
std::optional<uint64_t> PEnd =
944-
checkedAdd(POffset, PFilesz, "program header file end");
933+
checkedAddUint64(POffset, PFilesz, "program header file end");
945934
if (!PEnd)
946935
return false;
947936
if (POffset <= TextOffset && *PEnd >= *TextEnd) {
948937
std::optional<uint64_t> NewPFilesz =
949-
checkedAdd(PFilesz, TrampTotal, "program header file size");
938+
checkedAddUint64(PFilesz, TrampTotal, "program header file size");
950939
std::optional<uint64_t> NewPMemsz =
951-
checkedAdd(PMemsz, TrampTotal, "program header memory size");
940+
checkedAddUint64(PMemsz, TrampTotal, "program header memory size");
952941
if (!NewPFilesz || !NewPMemsz)
953942
return false;
954943
PFilesz = *NewPFilesz;
@@ -957,23 +946,23 @@ static bool adjustProgramHeaders(uint8_t *Elf, size_t ElfSize,
957946
std::memcpy(Ph + offsetof(Phdr, p_memsz), &PMemsz, sizeof(PMemsz));
958947
} else if (POffset > TextOffset) {
959948
std::optional<uint64_t> NewPOffset =
960-
checkedAdd(POffset, TrampTotal, "post-.text program offset");
949+
checkedAddUint64(POffset, TrampTotal, "post-.text program offset");
961950
if (!NewPOffset)
962951
return false;
963952
POffset = *NewPOffset;
964953
std::memcpy(Ph + offsetof(Phdr, p_offset), &POffset, sizeof(POffset));
965954
uint64_t PVaddr;
966955
std::memcpy(&PVaddr, Ph + offsetof(Phdr, p_vaddr), sizeof(PVaddr));
967956
std::optional<uint64_t> NewPVaddr =
968-
checkedAdd(PVaddr, TrampTotal, "post-.text program vaddr");
957+
checkedAddUint64(PVaddr, TrampTotal, "post-.text program vaddr");
969958
if (!NewPVaddr)
970959
return false;
971960
PVaddr = *NewPVaddr;
972961
std::memcpy(Ph + offsetof(Phdr, p_vaddr), &PVaddr, sizeof(PVaddr));
973962
uint64_t PPaddr;
974963
std::memcpy(&PPaddr, Ph + offsetof(Phdr, p_paddr), sizeof(PPaddr));
975964
std::optional<uint64_t> NewPPaddr =
976-
checkedAdd(PPaddr, TrampTotal, "post-.text program paddr");
965+
checkedAddUint64(PPaddr, TrampTotal, "post-.text program paddr");
977966
if (!NewPPaddr)
978967
return false;
979968
PPaddr = *NewPPaddr;
@@ -1050,7 +1039,7 @@ static bool adjustSymbolValues(uint8_t *Elf, size_t ElfSize,
10501039

10511040
uint64_t SymOffset = SymBytes - File.base();
10521041
std::optional<uint64_t> Value =
1053-
checkedAdd(Sym.st_value, TrampTotal, "post-.text symbol value");
1042+
checkedAddUint64(Sym.st_value, TrampTotal, "post-.text symbol value");
10541043
if (!Value)
10551044
return false;
10561045
uint64_t Value64 = *Value;
@@ -1091,8 +1080,8 @@ ElfView::growWithTrampolines(ArrayRef<Trampoline> Trampolines,
10911080
return nullptr;
10921081
}
10931082

1094-
std::optional<uint64_t> TextEnd =
1095-
checkedAdd(textOffset(), textSize(), "growWithTrampolines .text end");
1083+
std::optional<uint64_t> TextEnd = checkedAddUint64(
1084+
textOffset(), textSize(), "growWithTrampolines .text end");
10961085
if (!TextEnd || *TextEnd > InputSize) {
10971086
log() << "hotswap: error: growWithTrampolines: .text range exceeds input "
10981087
<< "ELF size.\n";

0 commit comments

Comments
 (0)