[PATCH] D124347: [BOLT] RFC: Add golang support

[Vladislav Khmelevsky via Phabricator via llvm-commits]

yota9 added a comment.

Some places are explained and marked by me that needs extra attention or some suggestions for re-implemetation



================
Comment at: bolt/include/bolt/Core/BinarySection.h:418
   }
+  void setOutputContents(uint8_t *Data, uint64_t Size) {
+    IsChanged = true;
----------------
The mechanism of in-place data section change  should be re-implemented, your suggestions are welcomed :)


================
Comment at: bolt/lib/Core/BinaryBasicBlock.cpp:649
 
+BinaryBasicBlock::iterator
+BinaryBasicBlock::storeInstructionMetadata(BinaryBasicBlock::iterator II) {
----------------
We need this, since some of the instructions annotations might be "locked". E.g. when dealing with gc inlined functions, we might inline only one instruction (e.g NOP) which will be removed by bolt. But this instruction has inline metadata that we need to have in order to restore inlining golang tables. So before removing such instruction we need to create NOP instruction on it's place that will store this metadata.


================
Comment at: bolt/lib/Core/BinaryEmitter.cpp:1148
+
+  for (auto &SectionName : BC.ExtraSectionsNames) {
+    ErrorOr<BinarySection &> Section = BC.getUniqueSectionByName(SectionName);
----------------
The extra sections creation and emission mechanism should be re-implemented, your suggestions are welcomed :)


================
Comment at: bolt/lib/Passes/Golang.cpp:1040
+  uint8_t *const SectionData =
+      (uint8_t *)mmap(nullptr, MmapSize, PROT_READ | PROT_WRITE,
+                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
----------------
The mmap is used since at this point we are unable to tell the final section size, we are allocating 8 times more of original space, which is more than enough. Since mmap allocates pages lazily we don't have extra memory consumption. However we probalbe should use ordinary new here


================
Comment at: bolt/lib/Passes/Instrumentation.cpp:192
   for (MCInst &NewInst : Instrs) {
+    BC.MIB->getOrCreateAnnotationAs<bool>(NewInst, "IsInstrumentation") = true;
     Iter = BB.insertInstruction(Iter, NewInst);
----------------
The golang has a sp bias table for each function. Since instrumentation uses the stack we need to mark inserted instructions so later in golang-postprocess and golang pass we will handle these instructions specially


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124347/new/

https://reviews.llvm.org/D124347