[PATCH] PR 16899: Update iterator when SLP vectorizer changes the instructions in the basic block

[Arnold Schwaighofer]

Committed as r188832.

Thanks,
Arnold

On Aug 20, 2013, at 1:27 PM, Yi Jiang <yjiang at apple.com> wrote:

> Arnold,
> 
> Thank you for this. I attached the new patch here. Since I am still working on get the commit access. Could you help to review and submit it? Thank you. 
> <Pr16899.new.patch>
> On Aug 20, 2013, at 8:59 AM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
> 
>> Yi,
>> 
>> there is another test case attached to the PR that still fails. Can you update your patch to include a fix for this test case, too? It is the same problem only in the loop above the one you already fixed in this patch. The loop that is iterating over the PHIs also needs to be careful about invalidating the iterator. Also see my reponse on http://llvm.org/bugs/show_bug.cgi?id=16899.
>> 
>> Thanks,
>> Arnold
>> 
>> On Aug 19, 2013, at 5:00 PM, Nadav Rotem <nrotem at apple.com> wrote:
>> 
>>> LGTM.  Small formatting suggestion:
>>> 
>>> +          //We would like to start over since some instructions are deleted
>>> +          //and the iterator may become invalid value
>>> 
>>> Please insert spaces after the comment mark and end the sentence with a period. 
>>> 
>>> Thanks,
>>> Nadav
>>> 
>>> 
>>> On Aug 19, 2013, at 4:53 PM, Yi Jiang <yjiang at apple.com> wrote:
>>> 
>>>> Hi, 
>>>> 
>>>> Please help to review the patch for PR16899. The reason of crash is that the instructions in the BB could be erased in the loop body and the iterator value is invalid. Thank Joerg for submitting a workaround on this in r188605. While  basically in this patch we would like to update iterator "it" and "e" and start over going though the basic block every time we find that the BB changed, in this case we may find more potential opportunities without invalidate the iterators. To avoid redundant check, We also create a SmallSet to record the instructions we have visited and make sure we do not check them twice.  Also the test case is included. Please see the patch for details and any comments are appreciated. Thank you. 
>>>> 
>>>> Files:
>>>> lib/Transforms/Vectorize/SLPVectorizer.cpp
>>>> test/Transforms/SLPVectorizer/X86/pr16899.ll
>>>> 
>>>> <pr16899.patch>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>