Abandoned in favour of https://reviews.llvm.org/D53408

Abandoned in favor of [[ https://reviews.llvm.org/D53408 | [PPC64] Long branch thunks. ]]

Reverted in https://reviews.llvm.org/rL344526 until we can figure this out.

I've reverted this for now.

This test is failing on the powerpc bots as well as hexagon and s390x:
http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/20815
http://lab.llvm.org:8011/builders/clang-hexagon-elf/builds/20534

In D49795#1254181, @ruiu wrote:

This patch seems a bit too intrusive to me, but I'm not sure if it is unavoidable (and thus this is a straightforward implementation) or there's another way to implement it.

Removed extra space in comment, and incorrect assert.

• Added an explanation of the expected prologue instructions and how the linker is supposed to modify them.
• Add check for overflow on the adjusted stack size
• Add check that the register operands are the expected ones.
• Changed global-entry to local-entry offset calculation to use new helper function.

Moved checking of OPT_toc_optimise until after the machine has been decided and made it default to on only for PowerPC64.

Thanks for doing this. I didn't realize we got the wide behavior by default with llvm-readelf.

Addressed initial comments.

Thanks for digging up that comment, its nice to know.

A bit more info here since the fact the call to _ZNKSt6locale2id5_M_idEv is a recursive call makes the situation a bit more muddy. I tested this with an older gcc version (5.4.0) and a newer version (7.3.1 from advanced toolchain 11). The newer compiler will emit a nop after a recursive call if the function is interposable, while the older compiler skips the nop. The linkers from the older toolchain (gold and bfd from binutils 2.26) both use plt stubs for the recursive call, and will rewrite the following nop to a toc-restore if the nop is there. I think lld behavior to emit a fatal error when trying to link _ZNKSt6locale2id5_M_idEv into a shared object is the correct one.

LGTM with a few minor comments about the test.

• Error when optimizing an updated form load or store.
• Add an option that can disable the toc optimization.
• Expand the testing.

ping

ping

I've picked this up from @syzaara. I'll post a revised patch shortly.

In D40425#1183223, @rcorcs wrote:

Except for the bug detected by the sanitizer, which seems to be related to the old pass manager, this patch is ready to be committed.

Overall I think the patch looks good, but we should expand the ppc64-toc-addis-nop.s test to cover all the different sequences this will affect (like loading bytes/half-word/word signed and unsigned etc).

Tested the above change out on my laptop and if it fixes the leak failure. I'm going to see if I can reproduce the problem in another pass and open a bugzilla if I can reproduce it.

@rcorcs i t looks like simply changing the order of the addRequired to put BPI before BFI fixes the reported leak:

diff --git a/lib/Analysis/CFGPrinter.cpp b/lib/Analysis/CFGPrinter.cpp
index 10be4e4..396f352 100644
--- a/lib/Analysis/CFGPrinter.cpp
+++ b/lib/Analysis/CFGPrinter.cpp
@@ -228,8 +228,8 @@ namespace {

I've been looking into the sanitizer build-bot failure and the address sanitizer detects a leak in the old-pass manager. One of my team members encountered this same problem before, where a seemingly unrelated change caused the address sanitize to detect a leak in the old pass manger. I am hoping to get a chance to look at it closer tonight to see if there is a real problem or not.

In rL336000#259399, @rcorcs wrote:

Hi,

I've fixed the graph printing in Polly.

Should I create a separate patch for Polly?

Thanks.

Thanks for doing this!

Unfortunately I had to revert this (https://reviews.llvm.org/rL336000) since there were several build-bot breaks.