- User Since
- Aug 18 2016, 4:39 AM (161 w, 3 d)
Fri, Sep 20
Add additional test cases and limit this patch to converting cases with appropriate zext/sext.
Wed, Sep 18
D67553 adds SimplifyFMAMul. We should probably also have them there. Also, could we make use of multiply by 0.0 here, with the required fast-math flags?
Tue, Sep 17
@reames I think this should be good to go in now?
Mon, Sep 16
Adjust comment and test case.
Sun, Sep 15
Sat, Sep 14
Updated with @reames' suggestion, thanks!
Fri, Sep 13
Thanks Eli, LGTM. This is in line with how we handle verification in many other places.
Thu, Sep 12
Thanks! Could you also add another test where we have a call to lifetime.end, followed by another defining store, followed by a load? This should not get optimized by this patch.
LGTM, thanks! I've just landed D67177, to avoid delays with this patch.
I've put up a patch for SCEVUmin D67177. I plan to land it tomorrow.
Sorry for not being more clear. I meant we need a test where the latch is not exiting, which without this patch will assert and with this patch will return nullptr. So please add a test for something like
Wed, Sep 11
Split off simplifications not requiring rounding from SimplifFMulInst as SimplifyFMAFMul.
I've committed an alternative fix rL371595. Thanks Eli! I might submit just the renaming/clarifications to moveLCSSAPhis
Tue, Sep 10
LGTM. It would be good if you could prefix the title of the commit with something like [LoopInfo]. In the future, please upload the diff with full context.
Add assertion that moved instructions do not have side-effects, update test checks.
Remove fmul matching.
Is there any chance you could provide the test case that shows the behavior? There might be something we could do in hasDedicatedExits to make it faster for large loops with a large number of exit blocks and successors.
Mon, Sep 9
Address review comments, thanks!
I *think* getVectorIntrinsicIDForCall uses TargetLibraryInfo. Looks like we are still including TargetLibraryInfo.h through some other path, but we probably could also get rid of that path.
I've uploaded a new patch D67367. It duplicates and moves the instructions required in the latch. We have to duplicate the instructions and update its users, if we want to support cases where, for example the condition, gets used in the inner loop body as well.
IMO it is fine to say pragma vectorize(disable) disables the vectorizer completely, including interleaving. @Meinersbur, what do you think? I think it would be good to make that clear in the commit message.
Remove unrelated test-case.
Fri, Sep 6
Thu, Sep 5
I think the current restrictions avoid any problems. We only allow zext, trunc and cmp instructions between the IV increment and the branch inst. If the final value is used outside the loop, that should be fine. A problem could be occur if any of the moved instructions is part of a reduction. I tried and it seems like we do not recognize reductions with trunc/zext chains at the moment, so we should be covered there. But I guess it would be good to make sure the instructions beside the IV increment are only used in the check for the exit condition.