[PATCH] D121392: [BOLT] Set cold sections alignment explicitly

Sat Mar 12 10:12:55 PST 2022

yota9 marked an inline comment as done.
yota9 added inline comments.

================
Comment at: bolt/lib/Core/BinaryEmitter.cpp:294-296
+    // Set section alignment to at least maximum possible object alignment.
+    // We need this to support LongJmp and other passes that calculates
+    // tentative layout.
----------------
yota9 wrote:
> rafauler wrote:
> > Why is this necessary? Is it possible to write a testcase that shows where we are getting the layout wrong?
> As I said cold text section alignment is set using the maximum alignment value
> passed to the emitCodeAlignment (see just under of these new lines). The thing is that when we calculate tentative layout in LongJmp currently we don't take into account that there will be the alignment gap between text and text.cold (see new line I've added there). In order to take cold text alignment in to the account we need to know the biggest alignment value we will pass to the emitCodeAlignment. So the simple idea here is to align the cold section ( which we didn't explicitly aligned before ) with the biggest possible value (opts::AlignFunctions). This way we are guaranteed that it will be the maximum possible value that might be passed to the emitCodeAlignment and now we can easily calculate tentative layout more precise. 
> I will try to create the simple test soon, I hope it won't be difficult. 
UPD: I've tried to think of the test, but as for aarch64 we now can trigger only the bl case. To do this we should write something, that will have between the call and callee ~128mb, to trigger the error after cold section will be created. We can use "rept" in asm, but it will be very slow (both codegen and bolt) and quite a big binary will be created. So I just don't see the reasonable and easy way to create such test currently..

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121392