- User Since
- Jan 29 2019, 6:22 PM (29 w, 4 d)
@DTharun Please add EXPECT_EQ(L->getLoopGuardBranch(), nullptr); to all test cases which has non simplified loops or non rotated loops in LoopInfoTest.cpp.
Thu, Aug 22
Wed, Aug 21
Add a test case with multiple loop exiting blocks.
Tue, Aug 13
@fhahn Most of the test cases in LoopInfoTest.cpp has checks for getLoopGuardBranch(). Are you talking about LoopUniqueExitBlocks and LoopNonLatchUniqueExitBlocks?
actually the check for preheader and latch should change back to an assert, as we already checked that the loop is in simplify form, so there must exists a preheader and latch.
Mon, Aug 12
Fri, Aug 9
@nikic As suggested, I added a flag to decide if to generate basic block names as variables.
Wed, Jul 31
Tue, Jul 30
Jul 25 2019
Jul 23 2019
@nikic Thanks for the suggestion. I personally ran into the issue that reassociate-after-unroll.ll failed due to the different in basic block name, after modifying the O2 pass pipeline. And modifying the script, was the suggested way to solve the failure. I am willing to put this change behind a flag if that's preferred, but it maybe hard for user to know when to use the flag, or even know the existence of it.
It depends on how other reviewers think about it...
Any thoughts? I prefer to keep the script clean and simple, although the one time change to the LIT tests will not be minimal.
Jul 22 2019
Next step after this lands is modifying Loop Fusion to use the loop guard. @kbarton
I cannot think of any clean way to identify labels, there are three ways that label can be:
- at the start of a basic block.
- follow by "label".
- incoming blocks in phinode.
Jul 20 2019
Changed this patch to only work on the simplest form.
Jul 17 2019
Jul 16 2019
Jul 15 2019
Removed the use of Dominator Tree, and removed isLoopGuardBranch() as the loop guard definition is now the same as getLoopGuardBranch().
@Meinersbur As discussed, I strengthened the definition of the loop guard which disallow conditional terminator instructions inside the guarded region outside of the loop of interest.
@Meinersbur @reames @etiotto Do you think this patch is now good enough to land as the first patch? We can loosen or strengthen the definition of the loop guard in later patches when applicable.
Jul 11 2019
Disallow blocks with non unique successors in the guarded region but outside of the loop of interest.
Jul 9 2019
Already committed on July 8 (7d8f30e6b2f27d55d4a14392951e4a61d7598767).
Addressed code review comments from @Meinersbur
Jul 8 2019
Jul 5 2019
@hfinkel Thanks for the review! I am not sure how to add a test in test/Analysis/LoopInfo, as cloneLoopWithPreheader() is not called by LoopInfo, so I added a test in unittests/Transforms/Utils/CloningTest.cpp instead.
Jul 4 2019
Jul 3 2019
I like your distinction between weak and proper guards. We probably need better terminology, but both are useful. For the moment, I'd suggest we start with the stronger form and add the weaker one later if needed.
Jun 29 2019
Strengthen the definition of loop guard by disallowing non safe instructions guarded by the branch but not in the loop.
@reames Thanks for the review! I agree that there are still much more to consider for how we want to define a loop guard, to make the most use of it.
Jun 27 2019
Jun 25 2019
Jun 24 2019
@Meinersbur Thanks for the review! What do you think of the updated changeset?
Jun 17 2019
Addressed all review comments from Michael.
Added the assert. Thanks for the reviews.
Jun 14 2019
Jun 10 2019
Jun 6 2019
Jun 5 2019
Jun 4 2019
Jun 3 2019
If there are no further comments or concerns, I will commit this patch on this Wed.
May 29 2019
@fhahn Thanks for your review. Do you think this is better now? Is there some areas you would like me to look into more?
May 28 2019
May 25 2019
Addressed all review comments.
May 24 2019
May 22 2019
May 17 2019
May 16 2019
May 15 2019
May 14 2019
May 13 2019