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.

In D40425#1147926, @rcorcs wrote:

I've just noticed that last time I uploaded the wrong diff.

This is the actual diff that fixes all conflicts with the latest LLVM version.

• clang formatted patch
• Switched the relax RelExpr to one that calculates a got-offset.
• used relocateOne to make it clearer what we were relaxing to.
• updated register mask to use hex instead of decimal.

Updated formatting, removed the 2 new RelExprs since they aren't needed, expanded comments on the relaxation specifics.

Other then the 1 comment this LGTM.

In D48090#1130092, @ruiu wrote:

Can you give me a pointer to the spec describing these relaxations?

LGTM

committed in https://reviews.llvm.org/rLLD334465

we have already have a patch that adds support for nearly all the DTPREL relocations here: https://reviews.llvm.org/D47596. Its been sitting for a while with-out a review though, rather then submit a new patch that adds support for only a few of the local-dynamic relocations I would appreciate a review on that so we can commit support for all at once.

ping

Updated tests to produce the missing dtprel/tprel relocation types.

This LGTM, but I think someone with more lld experience should Ok it as well.

Fixed extra space.

In D47379#1113896, @grimar wrote:

New naming is confusing IMO. We had R_GOTONLY_PC_FROM_END, R_GOT_FROM_END and R_GOTREL_FROM_END.
Since names contains "GOT" it is clear that "FROM_END" says about the end of GOT.

But R_TLSGD_FROM_END is different made me think about the end of TLS first of all.

commited in https://reviews.llvm.org/rL333200

In D47325#1111542, @ruiu wrote:

Generally looking good, it needs a bit more time to understand the test though.

Renamed the test and fixed several places where I used global-dynamic instead of`general-dynamic`.
Added run steps to the test to run on BE as well as LE.

Minor nit: the instructions that use this class are either Z23Form_7 ( xsrqpxp) or Z23Form_8 (xsrqpi, xsrqpix). The structure is correct but I think we should stick with the naming that matches the instruction descriptions in the ISA.

LGTM.

Addressed review comments.

Updated based on review comments.