[PATCH] D46635: [SimplifyCFG] Fix a debug invariant bug in FoldBranchToCommonDest()

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2559
+                    E = BB->instructionsWithoutDebug().end();
+               I != E;) {
             Instruction *Curr = &*I++;
----------------
dstenb wrote:
> vsk wrote:
> > Can this simply be written as: `for (auto &Curr : BB->instructionsWithoutDebug())`?
> > 
> As we may erase instructions, I don't think we can use a range-based for loop here.
Yep, unfortunately this is quite a widespread pattern which prevents us from using range-based loops.... Maybe there's something nicer/ more generic we could do in such cases, but I think that would require some further investigation.


================
Comment at: test/Transforms/SimplifyCFG/fold-branch-debuginvariant.ll:7
+
+; CHECK: bb1:
+; CHECK: and i1
----------------
Could you use CHECK-LABEL here and for bb2?


================
Comment at: test/Transforms/SimplifyCFG/fold-branch-debuginvariant.ll:8
+; CHECK: bb1:
+; CHECK: and i1
+; CHECK: bb2:
----------------
nit: IMO it is slightly clearer when matching the operands too: `and i1 false, false` would


Repository:
  rL LLVM

https://reviews.llvm.org/D46635