[PATCH] D141234: [BOLT][Golang support] Core changes

Sat Jan 28 06:05:38 PST 2023

yota9 added a comment.

I'm not sure what causes clang-format to complain about BF.h , locally I'm unable to repeat this.
Anyway I've commented about 3 main problems in this patch. I would be happy to discuss them and submit the golang support patches one-by-one, it would be the most right and simple away I think.
@maksfb @rafauler @Amir

================
Comment at: bolt/lib/Core/BinaryBasicBlock.cpp:651

+bool BinaryBasicBlock::storeInstructionMetadata(BinaryBasicBlock::iterator II) {
+  // In case the instruction has locked annotation we will save all
----------------
We need this, since some of the instructions annotations might be "locked". E.g. when dealing with golang 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/BinaryContext.cpp:2009

+BinarySection &BinaryContext::registerExtraSection(StringRef Name,
+                                                   unsigned ELFType,
----------------
Golang pass needs to create 2 extra read sections. We need to create PHDR for them and emit them properly to the binary. I'm not sure how to refactor the code connected to extra section handling, would be thankful for ideas.

================
Comment at: bolt/lib/Core/BinarySection.cpp:169
 BinarySection::~BinarySection() {
+  if (isChanged()) {
+    munmap(getOutputData(), getOutputSize());
----------------
This is currently used for the created by golang pass sections. They don't have input data, so it is used as a crutch where we mark the section as changed and use its output contents for emission. With the new sections creating/emission that is probable the second problem we need to discuss. 
P.S. The golang pass mmaps the memory with overhead since during section creation we're unable to table the final size. It is not a problem to use malloc/new, I've done it so the memory would be allocated lazily on the page access, but actually the sizes there are not so big..

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141234