LongJmp: Fix alignment issues

[yota9]

This patch fixes multiple problems:

• Instead of using opts::Align* we need to use per-function values
• The cold section is aligned to the maximum value that was passed to the emitCodeAlignment call. So we need to align DotAddress on tentativeLayoutRelocColdPart call and set alignment to the cold section in case we won't call emitCodeAlignment.
• Adjust PageSize if it is smaller then AlignFunctions value
• The rare case: Check is we can reach stub insertion point and relax instruction in place if we cannot.
• Small refactorings to speed-up pass execution: add breaks, add EstimatedTextSize under opts::UseOldText option

Vladislav Khmelevsky, Advanced Software Technology Lab, Huawei

[rafaelauler]

Hi Vladislav! Can you elaborate on why do we need LongJmp running on X86? This is a very expensive pass and we should consider alternative solutions first, if possible.

It would help to have test cases showing the problem being solved so we can work on a solution together. I feel the PR, currently, is missing other changes and context to demonstrate what is being solved. For example, by the looks of it, I couldn't find a change that actually activated this pass for X86, right?

[yota9]

Hello @rafaelauler ! This pass will be used with golang support. The problem with golang that we need exactly byte-to-byte binary reading from the source file and need byte-to-byte layout of the new binary before emitting it. So LLVM should not relax instructions by it's own after golang pass is finished. I've added longjmp support for x86 just recently, before that I've just relaxed all branches to long jumps in order to work with golang. So it will be activated only for golang :) P.S. This patch also solved couple of issues in with this pass in order to make it more accurate.

[yota9]

@rafaelauler Actually it is not very expensive for x86, since here we rely on instruction relaxation. So no stubs are inserted and no fixbranches are needed easier. I didn't fix shortJmp/longJmp creation for x86, since +- 2Gb are seems to be enough for us currently :)

[rafaelauler]

Thanks for explaining it! How hard is it to add a test case that showcases this functionality being used? Also, it it possible to add to this PR the missing bits that will actually turn it on this pass on X86?

If I understood correctly, there is another pass that uses this, and that's why this feels, as it is, incomplete. What if we land first the module that uses this? Or maybe merge everything in the same PR? I don't have a problem with having a larger PR, as long as it is doing one thing that can be tested with a test case, and no dead code is being added.

[rafaelauler]

Another thing we can do, instead of doing one larger PR, is to open a new PR with a commit on top of this one, showing how this is used. But ideally, we would have a "self-contained" PR, that is, a set of changes that we can test with a test case.

[yota9]

@rafaelauler Sure, let me remove x86 support (e.g. instruction relaxation), there are enough fixes not related to architecture support :)

[rafaelauler]

Sounds good!

[yota9]

Hello @rafaelauler ! I've reverted the x86 specifics. Not completely, like the "InstSize" in getTargetOffset stays unused for now, but I dont't think it is a big deal, the rest of the code I've removed will be submitted with golang support.

[rafaelauler]

I'm getting a conflict on BinaryEmitter.cpp at the moment, can you rebase this?

PS: Use git -c merge.renamelimit=230 rebase (or cherry-pick, depending on your workflow)

[yota9]

@rafaelauler Sure, done

[HShan886]

Can this pr fix the core in #206 ?

[yota9]

@Haishan312 You should check it :) P.S. Don't use align-blocks, it is unsupported and not related to this PR.

[HShan886]

@Haishan312 You should check it :) P.S. Don't use align-blocks, it is unsupported and not related to this PR.

But, I still get a failure information "Assertion `isInt<21>(BranchImm)' failed". and it is same to #206 My binary is 602.gcc_s