[PATCH] D33850: Inlining: Don't re-map simplified cloned instructions.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

I forgot to mention this: this needs a test case that would fail/crash before the patch. =]



================
Comment at: lib/Transforms/Utils/CloneFunction.cpp:457
   SmallVector<const PHINode*, 16> PHIToResolve;
-  for (const BasicBlock &BI : *OldFunc) {
+  for (const BasicBlock &BI : make_range(OldFunc->begin(), OldFunc->end())) {
     Value *V = VMap.lookup(&BI);
----------------
iteratee wrote:
> eraman wrote:
> > Unrelated change?
> Not unrelated. If you're inlining a function into itself, end() changes. I can make it a separate patch if you'd like, but I put it in this patch on purpose.
I'd make it a separate patch (or update the patch description to talk about it).

But I also don't think this is quite correct -- even the new version computes the end iterator once and caches it, so I'm not sure how this fixes the issue cited.


Repository:
  rL LLVM

https://reviews.llvm.org/D33850