Hi, I'm studying loop vectorizer. I don't understand the code yet. But it looks not right to assign L->begin() to E. Is it a typo? Thanks, Liang diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp b/lib/Transforms/Vectorize/LoopVectorize.cpp index 435c005..87b5d79 100644 --- a/lib/Transforms/Vectorize/LoopVectorize.cpp +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp @@ -1067,7 +1067,7 @@ struct LoopVectorize : public FunctionPass { // We only handle inner loops, so if there are children just recurse. if (!L->empty()) { bool Changed = false; - for (Loop::iterator I = L->begin(), E = L->begin(); I != E; ++I) + for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I) Changed |= processLoop(*I); return Changed; }
Almost certainly, yes. Nice catch! On Mar 18, 2014, at 2:38 PM, Liang Wang <netcasper at gmail.com> wrote:> Hi, > > I'm studying loop vectorizer. I don't understand the code yet. But > it looks not right to assign L->begin() to E. Is it a typo? > > Thanks, > Liang > > diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp > b/lib/Transforms/Vectorize/LoopVectorize.cpp > index 435c005..87b5d79 100644 > --- a/lib/Transforms/Vectorize/LoopVectorize.cpp > +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp > @@ -1067,7 +1067,7 @@ struct LoopVectorize : public FunctionPass { > // We only handle inner loops, so if there are children just recurse. > if (!L->empty()) { > bool Changed = false; > - for (Loop::iterator I = L->begin(), E = L->begin(); I != E; ++I) > + for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I) > Changed |= processLoop(*I); > return Changed; > } > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Looking at it now, curious why no tests failed. On Tue, Mar 18, 2014 at 2:48 PM, Jim Grosbach <grosbach at apple.com> wrote:> Almost certainly, yes. Nice catch! > > > On Mar 18, 2014, at 2:38 PM, Liang Wang <netcasper at gmail.com> wrote: > > > Hi, > > > > I'm studying loop vectorizer. I don't understand the code yet. But > > it looks not right to assign L->begin() to E. Is it a typo? > > > > Thanks, > > Liang > > > > diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp > > b/lib/Transforms/Vectorize/LoopVectorize.cpp > > index 435c005..87b5d79 100644 > > --- a/lib/Transforms/Vectorize/LoopVectorize.cpp > > +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp > > @@ -1067,7 +1067,7 @@ struct LoopVectorize : public FunctionPass { > > // We only handle inner loops, so if there are children just recurse. > > if (!L->empty()) { > > bool Changed = false; > > - for (Loop::iterator I = L->begin(), E = L->begin(); I != E; ++I) > > + for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I) > > Changed |= processLoop(*I); > > return Changed; > > } > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140318/a0a94227/attachment.html>
Apologies, this was dead code left over from the first half of a refactoring I did. I've tidied it up and clarified matters with an assert in r204184. Sorry for any confusion. On Tue, Mar 18, 2014 at 2:38 PM, Liang Wang <netcasper at gmail.com> wrote:> Hi, > > I'm studying loop vectorizer. I don't understand the code yet. But > it looks not right to assign L->begin() to E. Is it a typo? > > Thanks, > Liang > > diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp > b/lib/Transforms/Vectorize/LoopVectorize.cpp > index 435c005..87b5d79 100644 > --- a/lib/Transforms/Vectorize/LoopVectorize.cpp > +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp > @@ -1067,7 +1067,7 @@ struct LoopVectorize : public FunctionPass { > // We only handle inner loops, so if there are children just recurse. > if (!L->empty()) { > bool Changed = false; > - for (Loop::iterator I = L->begin(), E = L->begin(); I != E; ++I) > + for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I) > Changed |= processLoop(*I); > return Changed; > } > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140318/55a0989f/attachment.html>
And by r204184, I actually mean r204187 because I misfired my git client. Sorry. On Tue, Mar 18, 2014 at 3:00 PM, Chandler Carruth <chandlerc at google.com>wrote:> Apologies, this was dead code left over from the first half of a > refactoring I did. I've tidied it up and clarified matters with an assert > in r204184. Sorry for any confusion. > > > On Tue, Mar 18, 2014 at 2:38 PM, Liang Wang <netcasper at gmail.com> wrote: > >> Hi, >> >> I'm studying loop vectorizer. I don't understand the code yet. But >> it looks not right to assign L->begin() to E. Is it a typo? >> >> Thanks, >> Liang >> >> diff --git a/lib/Transforms/Vectorize/LoopVectorize.cpp >> b/lib/Transforms/Vectorize/LoopVectorize.cpp >> index 435c005..87b5d79 100644 >> --- a/lib/Transforms/Vectorize/LoopVectorize.cpp >> +++ b/lib/Transforms/Vectorize/LoopVectorize.cpp >> @@ -1067,7 +1067,7 @@ struct LoopVectorize : public FunctionPass { >> // We only handle inner loops, so if there are children just recurse. >> if (!L->empty()) { >> bool Changed = false; >> - for (Loop::iterator I = L->begin(), E = L->begin(); I != E; ++I) >> + for (Loop::iterator I = L->begin(), E = L->end(); I != E; ++I) >> Changed |= processLoop(*I); >> return Changed; >> } >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140318/3fc5c33c/attachment.html>