[PATCH] D129107: [BOLT][HUGIFY] adds huge pages support of PIE/no-PIE binaries

[Rafael Auler via Phabricator via llvm-commits]

rafauler added inline comments.


================
Comment at: bolt/runtime/hugify.cpp:88-90
+static void hugifyForOldKernel(uint8_t *From, uint8_t *To,
+                               uint8_t *FromAlignedPage,
+                               uint8_t *ToAlignedPage) {
----------------
I think we can just use the old behavior here (work with two pointers From, To - they're both aligned to the page, we don't ever need to know the original hot sizes in this function. This makes this function more confusing to read.


================
Comment at: bolt/runtime/hugify.cpp:112
+  // Copy the hot code to a temporary location.
+  memcpy(Mem, From, Size);
 
----------------
Mistake is probably here, copying less bytes than are actually needed, program will crash.


================
Comment at: bolt/test/runtime/X86/user-func-reorder.c:33
 RUN: llvm-bolt %t.exe --relocs=1 --lite --reorder-functions=user \
-RUN:   --hugify --function-order=%p/Inputs/user_func_order.txt -o %t
+RUN:   --hugify=4.18 --function-order=%p/Inputs/user_func_order.txt -o %t
 RUN: llvm-nm --numeric-sort --print-armap %t | \
----------------
You don't need 4.18 option here. The reason this test is failing is because in 5.10, you're not copying all contents of the page. You can't just copy hot contents. You need to restore the old behavior that will copy the entire page, and not just the first bytes that correspond to hot code.

In other words, as it is, the 5.10 behavior is broken. I left comments in the function where I suspect the problem is from the last time I debugged this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129107