[PATCH] D28522: Codegen: Make chains from lattice-shaped CFGs

[Kyle Butt via Phabricator via llvm-commits]

iteratee added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1296
 
+  // if BB is part of a lattice, Use the lattice to determine the optimal
+  // fallthrough edges
----------------
davidxl wrote:
> iteratee wrote:
> > davidxl wrote:
> > > Extract the special handling of lattice code into a helper to make the main flow of the caller cleaner.
> > It already is in 2 helpers. Ignoring the logging the logic is:
> > if (isLattice)
> >   return getBestLattice();
> > 
> > I don't see how a helper would be useful in this case.
> I suggest pushing isLattice check into getBestLatticeSuccessor call.  Also pushing the debug tracking code there too. The caller will be like
> 
> if (LatticeSucc = getBestLatticeSuccessor(...)) {
>    return BlockAndTailDupResult(LatticeSucc, false}'
>  }
> 
> 
Keeping them separate makes the early return logic simpler, but I moved the debugging code.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1292
+    DEBUG(dbgs() << "Lattice, but not one of the chosen edges.\n");
+    return BestSucc;
+  }
----------------
davidxl wrote:
> Is this null? can you explain this code?
Yes. If BB is part of a lattice, but not an optimal edge, then we return early. We've already determined that all of BB's successors have a better fallthrough predecessor.


https://reviews.llvm.org/D28522