- User Since
- Jan 29 2019, 6:22 PM (37 w, 5 d)
Sun, Oct 6
Thu, Sep 26
Wed, Sep 25
Any reason not to use Loop::getExitBlock()? The behavior is different when there are are multiple exiting edges to the same block, but does it matter? Maybe we should change Loop::getExitBlock() to only return nullptr if two different exit blocks exist, as e.g. getLoopPredecessor() already does.
Tue, Sep 24
Sep 20 2019
Sep 19 2019
Sep 18 2019
Sep 12 2019
or you can add EXPECT_EQ(L->getLoopGuardBranch(), nullptr); to existing tests, e.g. LoopWithSingleLatch....in that test case, you can see that the loop is not rotated, i.e. the latch for.inc is not the exiting block, the exiting block is for.cond.
Sep 11 2019
Sep 6 2019
Aug 30 2019
Aug 29 2019
Aug 28 2019
Aug 23 2019
@DTharun Please add EXPECT_EQ(L->getLoopGuardBranch(), nullptr); to all test cases which has non simplified loops or non rotated loops in LoopInfoTest.cpp.
Aug 22 2019
Aug 21 2019
Add a test case with multiple loop exiting blocks.
Aug 13 2019
@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.
Aug 12 2019
Aug 9 2019
@nikic As suggested, I added a flag to decide if to generate basic block names as variables.
Jul 31 2019
Jul 30 2019
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.