[llvm] r291973 - [GlobalISel] track predecessor mapping during switch lowering.

Kristof Beyls via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 00:00:19 PST 2017


A few nitpicks - apologies for not reviewing in time for pre-commit.

> On 14 Jan 2017, at 00:11, Tim Northover via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h?rev=291973&r1=291972&r2=291973&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h Fri Jan 13 17:11:37 2017
> @@ -70,6 +70,9 @@ private:
>   // lives.
>   DenseMap<const BasicBlock *, MachineBasicBlock *> BBToMBB;
> 
> +  typedef std::pair<const BasicBlock *, const BasicBlock *> CFGEdge;
> +  DenseMap<CFGEdge, SmallVector<MachineBasicBlock *, 1>> MachinePreds;
> +

I think having a  comment for MachinePreds would help understand this code.
Maybe something like """One BasicBlock can be translated to multiple MachineBasicBlocks.
For such BasicBlocks translated to multiple MachineBasicBlocks, MachinePreds retains
a mapping between the edges arriving at the BasicBlock
to the corresponding created MachineBasicBlocks. Some BasicBlocks that get translated
to a single MachineBasicBlock may also end up in this Map."""?
Note that I may not have fully understood this code - so the comment also checks my understanding ;)

> Modified: llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp?rev=291973&r1=291972&r2=291973&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp (original)
> +++ llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp Fri Jan 13 17:11:37 2017
> @@ -12,6 +12,7 @@
> 
> +void IRTranslator::addMachineCFGPred(CFGEdge Edge, MachineBasicBlock *NewPred) {
> +  assert(NewPred && "new edge must be a real MachineBasicBlock");
s/edge/predecessor/ ?




More information about the llvm-commits mailing list