- User Since
- Aug 18 2016, 4:39 AM (109 w, 4 d)
Fri, Sep 21
Thu, Sep 20
LGTM. Could you please add a unittest similar to the one added in D40985?
Move the lifetime handling before cloning the instruction. The cases where we delete the clone do not apply to lifetime intrinsics anyways, so there is no need to clone it in that case.
Rebased and added comments.
I think this fixes a longstanding bug report PR15376.
LGTM thanks! It would be great if you could also add a simple test case.
LGTM, with my previous comments.
I simplified it. I am not sure how I can get rid of va_list; what i am testing is va_start calledin the caller and va_end in the callee. I need to have va_list to pass it from the caller to the callee.
Wed, Sep 19
It's probably worth waiting with committing until tomorrow, in case Eli has any more suggestions
Tue, Sep 18
Sun, Sep 16
Fri, Sep 14
Thu, Sep 13
Further testing surfaced a few cases where ScalarEvolution and LCSSA are not preserved properly. I'll fix those issues first.
Wed, Sep 12
FWIW, it seems like OrderedBB invalidation is causing bugs at least in LoopSafetyInfo (D50377) which @mkazantsev is currently working on fixing. There might be other places that get this wrong too, so having automatic invalidation seems like another good plus on top of the speedups.
Tue, Sep 11
Strengthen test checks
Thanks! I've rebased the patch and will wait for a bit, in case there are any additional concerns, please let me know.
D51664 gets rid of building OrderedBasicBlocks and comes with its own validation of the ordering.
Mon, Sep 10
Fri, Sep 7
Thank you very much for having a look so quickly!
Adjusted comment and simplified test case.
This is not ready for review yet, I'll continue with this if D51505 is submitted.
IIUC by always adding to the end of WorkListVec, we could end up using substantial more memory, depending on the input program. In practice it shouldn't be too bad, as I think an upper bound for the times an instruction is added to WorkListVec is the number of operands (it will only get added if one of its operands gets constant folded, and once an operand is constant folded, it won't get folded again). And usually most instructions have a small number of operands.
Thu, Sep 6
Rebased after rL341537
Wed, Sep 5
LGTM with some more nits to the test cases. The EnableVPlanNativePath sprinkled around are not ideal, but I don't think there is a better alternative and we should be able to get rid of them as we go along. Please wait a few days with submitting this, to give other people a change to raise any additional concerns.
Tue, Sep 4
I just tried to build your patch, and it triggers
lib/Transforms/Vectorize/LoopVectorize.cpp:1288: bool llvm::LoopVectorizationCostModel::isScalarAfterVectorization(llvm::Instruction*, unsigned int) const: Assertion `ScalarsPerVF != Scalars.end() && "Scalar values are not calculated for VF"' failed.
Mon, Sep 3
Great, thanks for the review!
Ok thanks! So I can commit this patch as is?
updated merging of unknown and notconstant values to go to overdefined
Fri, Aug 31
Thu, Aug 30