[PATCH] CodeGenPrep: rewrite a few loops in C++11 style

[Duncan P. N. Exon Smith]

> On 2015-Jan-07, at 15:27, Ramkumar Ramachandra <artagnon at gmail.com> wrote:
> 
> Revert first hunk, and change the cursor to BasicBlock in the second
> and third loops. Do one additional inner loop.
> 
> 
> http://reviews.llvm.org/D6868
> 
> Files:
>  lib/CodeGen/CodeGenPrepare.cpp
> 
> Index: lib/CodeGen/CodeGenPrepare.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenPrepare.cpp
> +++ lib/CodeGen/CodeGenPrepare.cpp
> @@ -261,9 +261,9 @@
>   if (!DisableBranchOpts) {
>     MadeChange = false;
>     SmallPtrSet<BasicBlock*, 8> WorkList;
> -    for (Function::iterator BB = F.begin(), E = F.end(); BB != E; ++BB) {
> -      SmallVector<BasicBlock*, 2> Successors(succ_begin(BB), succ_end(BB));
> -      MadeChange |= ConstantFoldTerminator(BB, true);
> +    for (BasicBlock &BB : F) {
> +      SmallVector<BasicBlock*, 2> Successors(succ_begin(&BB), succ_end(&BB));
> +      MadeChange |= ConstantFoldTerminator(&BB, true);

This change LGTM.

>       if (!MadeChange) continue;
> 
>       for (SmallVectorImpl<BasicBlock*>::iterator
> @@ -4327,18 +4327,17 @@
> // find a node corresponding to the value.
> bool CodeGenPrepare::PlaceDbgValues(Function &F) {
>   bool MadeChange = false;
> -  for (Function::iterator I = F.begin(), E = F.end(); I != E; ++I) {
> +  for (BasicBlock &BB : F) {

This change LGTM.

>     Instruction *PrevNonDbgInst = nullptr;
> -    for (BasicBlock::iterator BI = I->begin(), BE = I->end(); BI != BE;) {
> -      Instruction *Insn = BI; ++BI;
> -      DbgValueInst *DVI = dyn_cast<DbgValueInst>(Insn);
> +    for (Instruction &Insn : BB) {
> +      DbgValueInst *DVI = dyn_cast<DbgValueInst>(&Insn);

This change looks incorrect.  You've effectively moved the timing of the
increment.  The code that follows invalidates the iterator at
`DVI->removeFromParent()`:

      Instruction *Insn = BI; ++BI;
      DbgValueInst *DVI = dyn_cast<DbgValueInst>(Insn);
      // Leave dbg.values that refer to an alloca alone. These
      // instrinsics describe the address of a variable (= the alloca)
      // being taken.  They should not be moved next to the alloca
      // (and to the beginning of the scope), but rather stay close to
      // where said address is used.
      if (!DVI || (DVI->getValue() && isa<AllocaInst>(DVI->getValue()))) {
        PrevNonDbgInst = Insn;
        continue;
      }

      Instruction *VI = dyn_cast_or_null<Instruction>(DVI->getValue());
      if (VI && VI != PrevNonDbgInst && !VI->isTerminator()) {
        DEBUG(dbgs() << "Moving Debug Value before :\n" << *DVI << ' ' << *VI);
        DVI->removeFromParent();


>       // Leave dbg.values that refer to an alloca alone. These
>       // instrinsics describe the address of a variable (= the alloca)
>       // being taken.  They should not be moved next to the alloca
>       // (and to the beginning of the scope), but rather stay close to
>       // where said address is used.
>       if (!DVI || (DVI->getValue() && isa<AllocaInst>(DVI->getValue()))) {
> -        PrevNonDbgInst = Insn;
> +        PrevNonDbgInst = &Insn;
>         continue;
>       }
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D6868.17876.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits