This LGTM

This LGTM

Hello, sorry but can you please revert this commit and recommit it when you have a fix or work around that doesn't break our bots:
It breaks http://lab.llvm.org:8011/builders/ppc64le-lld-multistage-test/builds/11228 which builds with -Werror
Please also note that it introduced 103 warnings in clang-ppc64le-linux-multistage/builds/13042

Rebasing and updating patch to use TLS enablement option.

Rebasing patch on D85448 and editing test cases to use enablement option.

Replacing assert() with report_fatal_error()

Rebasing on most recent D81947 and addressing review comments:

• rebasing on the updated version of D81947
• updating test cases according to review comments
• updating comments in code
• updating llvm-readobj run lines to follow new interface

Rebasing and addressing review comments:

• rebased on the updated version of D82315
• updated test cases according to reviews
• updated llvm-readobj run lines in test cases to reflect interface change

Addressing review comments and updating test cases:

• rebasing the patch on to a more recent master and fixed merge conflicts
• changed test cases to remove unnecessary bloat and update run lines.

LGTM

LGTM

This is LGTM, one suggestion on the clang-format change that can be done pre-commit.

This LGTM

• rearranging conditional logic for emitting MCSymbolRefExpr::VK_PPC_TLS_PCREL so that it is less redundant and reads better.
• replacing future with pwr10 in test cases and adding better comments.

• Renaming test functions names as per review comments.

• Rebasing branch to more recent version of master.
• Adding asserts to ensure values are always what they are expected to be.
• Code formating and comments added.
• Conditional logic for register operands rearranged to avoid potential bugs in PPCTLSDynamicCall.cpp.

• Rebasing patch to be dependant on D82315 - TLS Support for General Dynamic.
• Adding Combination flag MO_GOT_TPREL_PCREL_FLAG to more explicitly define the king that will output @got@tprel@pcrel relocation.
• Adding assembly reader test cases by request of reviewer for more robust testing.
• Minor format changes.

Addressing clang format suggestion.

Adding a combination flag to more explicitly set the bits needed to produce
General Dynamic TLS model relocations.

Addressing review comments.

Updating test case.

In D79747#2072011, @fhahn wrote:

I think it would be great to add more details about the benefit of the outlining to the description. For the example in the test case, outlining seems quite heavy handed. Wouldn't it be simpler and faster in those cases to just duplicate the return block?

Updating patch with author's credits.

Third attempt to address clang-format.

Addressing clang-format suggestion.

Address clang-format suggestions.

Updating this patch to make it a target independent IPO module pass.

Addressed some review comments.

While using Arcanist I accidentally made the differential change BASE because the recorded HEAD commit hash on phabricator somehow became mixed-matched with my local HEAD that i use for this branch.
I am manually uploading the diff to correct this.

Addressing review and clang-tidy comments

Addressing review and clang-tidy comments

LGTM, one pre-commit nit with the indentation.

I think the index for the StoreSpillOpcodesArray and LoadSpillOpcodesArray arrays are incorrect.

In D79747#2030808, @shchenz wrote:

1: I like the idea about reducing csr cost in prologue/epilogue by outlining some function bodies. Any special reason for applying this pass only on PowerPC? I think many other platforms also can benefit from this pass.

I can definitely do this, I'll mark this patch as "changes planned" and then post the updated target independent version when it is completed.

2: Is there any plan to extend the light path definition. Currently in this pass a light path is a path from entry block directly to exit block. But I think light path can be more general. A simple example

//   ENTRY
//     |
//    BB1 
//    / \    
//   |  BB2    
//   |   | \     
//   |  ANY BLOCKS   
//   |  /                   
//   RET
//

A light path for this case can be ENTRY->BB1->RET and outline other blocks.

If we plan to extend light path definition, I think better to do a little change in the code framework. Eg: define a function like findLightPath() in tryOutlining instead of hard code the light path?

There are plans to improve this pass once that pass is completed and makes it into the compiler. Thank you for this guidance, I will keep it in mind.

3: After get the light path, I think it’s better to call interfaces in class CodeExtractor to do outlining like hotcoldsplitting pass and PartialInliner pass did.

I had not considered this , I will investigate this possibility as well.

This LGTM, one minor comment about test case explanation, it can be done pre-commit for sure.

Sorry I missed that git clang-format changed whitespace on unrelated lines.
Pulling out simply because of this seems like unnecessary churn for the bots
especially since the formatting is an improvement.
If someone would like me to pull it, I certainly don't mind.
I will definitely be more vigilant next time.

I commented on the commit b771c4a8, FYI

Sorry I missed that git clang-format changed whitespace on unrelated lines.
Pulling out simply because of this seems like unnecessary churn for the bots
especially since the formatting is an improvement.
If someone would like me to pull it, I certainly don't mind.
I will definitely be more vigilant next time.

In D74486#1972299, @thakis wrote:

Looks like this breaks tests on Windows: http://45.33.8.238/win/12548/step_11.txt

Sorry to report but this is causing failures as well.
http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1695
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/35741

Testing Time: 760.34s
********************
Unresolved Tests (1):
    Profile-powerpc64le :: instrprof-merging.cpp

In D70930#1847950, @dschuff wrote:

I won't have time to look at it today; i'll just revert and take a look tomorrow.

@dschuff please note this patch causes build failures for PowerPC big endian buildbots with commit rGa928d127a52a
If the solution isn't a simple fix please revert the commit so we may continue testing the rest of the repository

In D72679#1832424, @epastor wrote:

In D72679#1832296, @kamaub wrote:

@epastor The lastest commit for this patch: r5f6dfa800e0b causes build failures on:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23101
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/43182
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/34405

@kamaub The build failures you see should have been fixed by the followup commit r6ccebe004446.

EDIT: On inspection, (for example) http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/43185 shows that you're now seeing a different error that appears unrelated?

@epastor The lastest commit for this patch: r5f6dfa800e0b causes build failures on:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23101
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/43182
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/34405