I'm working with the IBM legal team to see if there is any way we can get an exception or workaround for the copyright (these probably aren't the correct legal terms, but hopefully people understand the intention). I can update here once I have more information.

• Add const to all parameters of createPPCInstructionSelector.
• Address review comments - minor clean in PPC::lowerFormalArguments and update comments.
• Put all new GlobalISel related files into new GISel subdirectory.
• Fix warnings from clang-tidy and clang-format.
• Remove unnecessary whitespace.

Fix warnings from clang-tidy and clang-format and rebase to latest version of master.

LGTM

In D83100#2134249, @tschuett wrote:

Pardon my cmake, but don't you need a CMakeLists.txt in the GISel sub-directory?

• Put all new GlobalISel related files into new GISel subdirectory.

In D83100#2130726, @tschuett wrote:

AArch64 has a sub-directory for GlobalISel related things:
https://reviews.llvm.org/D81116

• Address review comments - minor clean in PPC::lowerFormalArguments and update comments.

I had some problems refreshing the patch using arcanist. I *think* I have fixed everything now, but if things look odd please let me know.

Refreshing diff

Refreshing diff.

Add const to all parameters of createPPCInstructionSelector.

Rebase based on latest version of master.

I looked through this and it seems fine to me.
I'm not clear about the extraneous frin in vector-constrained-fp-intrinsics.ll, but if it is extraneous I agree we can proceed with this and deal with it in a subsequent patch.

@Meinersbur I missed the RFC and discussion on the cfe-dev mailing list. Could you post a link here so that it's included in the history?

One minor comment that I just noticed - can you please change the names of the test cases to use _ instead of - (to be consistent with the other tests in the directory)?
Aside from that, LGTM.

Hi Diego,
Thank you for submitting this patch!
I think this fix makes sense. If you could go ahead and add the check rules for the test cases that would be great!

In D73129#1858261, @Meinersbur wrote:

In D73129#1844566, @Whitney wrote:

Option 1: Use LoopFusion infrastructure to jam innerloops recursively.
Option 2: Prove safety beforehand, and unroll and jamming without creating new loops.

IMHO option 2 is preferable and more robust.

To me the grouping that you have here makes the most sense, but I don't have a strong opinion either way.

I went through the recordings you cited, and as you expected these are mistakes.
First, the comment I made about loop latch having a single predecessor was a typo introduced when converting slides. That should have been successor, to match with the slides I presented at EuroLLVM.
The comment about single successor I got from the original patch for loop rotation (https://reviews.llvm.org/D22630). However, as @Meinersbur pointed out earlier, this comment doesn't make sense because the latch has to have two successors: the loop header and the loop exit block.

Hi @baziotis. First, thank you very much for working on this - I agree that more description of loop rotation is very important (and badly needed).
Regarding the specific questions you ask, I don't recall off hand, but I will echo @Meinersbur comment that it's possible we made mistakes in those presentations also. I'll try to review/remember what we were thinking in those presentations and reply here tomorrow.

I think this is a good change. My only question is with the test case. If we can get the necessary information out of MIR, then I think it's better to write an MIR test case instead of relying on the debug output from the machine scheduler.

LGTM.
I apologize for the delay in getting to this.

Could you please rebase this patch?
It no longer applies cleanly, since you committed https://reviews.llvm.org/D70768.

LGTM.

Addressed all the review comments.
I'm ready to land this, unless @Whitney has concerns about the test case.

The problem I had with a new subsection was getting "out" of the new subsection for the rest of the instructions under "How to Submit a Patch" (hopefully that makes sense).

I'm not sure what you mean here.

• Return false if no latch block exists, instead of assert (based on suggestion from Meinersbur)
• Add negative test case for loop that is not rotated.

Thanks for the review @delcypher!

• Move description of clang-format to Contributing.rst and add a link to Phabricator.rst to reference it.

LGTM.

LGTM.

Cancelling. Will post another version during the tutorial.

LGTM.
Thanks for adding the comment.

I don't remember why exactly this was necessary.
Looking at the current definition in LoopInfo.h, it's not immediately obvious to me either.
Could you either update the definition of getLoopGuardBranch in LoopInfo.h to state this case is not handled (and why). Or, if it's something obvious that I'm not seeing, expand the description here to make it clear?

Are there any other comments on this?
If not, I'd like to get this committed and start working on the next patch.

Sorry, I clicked submit before adding general comments.
Thanks for working on this. I think this looks OK, just needs some minor cleaning wrt coding guidelines and the test cases.
I'd like other people weigh in though, as it's been a while since I looked at SD Chains.

• Assert guard branch is conditional in getNonLoopBlock.

In D65464#1617929, @Meinersbur wrote:

Is there a long-term plan to avoid the code duplication? E.g. refactoring/keeping only the fuseGuardedLoops variant when loop guards become a normal form? One could also add some preprocessing that makes the two loop guards guard both loops, then use the standard fuse code.

• Address review comments.
• Improve haveIdenticalGuards method by ensuring the guards gave the same flow into/around the loop in addition to identical compare instructions.
• Remove (redundant) increments of FuseCounter statistic.
• Addressed review comments.
• Remove old, out of date comments in fuseGuardedLoops method.

• Addressed review comments.

• Remove (redundant) increments of FuseCounter statistic.

• Improve haveIdenticalGuards method by ensuring the guards gave the same flow into/around the loop in addition to identical compare instructions.

• Address review comments.