[llvm-commits] [llvm] r161463 - in /llvm/trunk: include/llvm/CodeGen/MachineBasicBlock.h include/llvm/CodeGen/MachineBranchProbabilityInfo.h lib/CodeGen/MachineBasicBlock.cpp lib/CodeGen/MachineBranchProbabilityInfo.cpp

[Jakob Stoklund Olesen]

On Aug 7, 2012, at 7:01 PM, Chandler Carruth <chandlerc at google.com> wrote:

> Can you send me a test case for the quadratic behavior? I'm happy to help with fixing it
> 

Attaching z16.ll from consumer-typeset. It crashes when I apply the quadratic algorithm fix.



It looks like edge weights aren't handled correctly when there are duplicate entries for a basic block in the successor list.

This is the quadratic loop:

  for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
       E = MBB->succ_end(); I != E; ++I) {
    uint32_t Weight = getEdgeWeight(MBB, *I);
    Sum += Weight;
  }

The getEdgeWeight(Src, Dst) function does a linear search: std::find(Src->succ_begin(), Src->succ_end(), Dst), making the loop quadratic in MBB->succ_size().

MachineBasicBlock can have duplicate successor entries:

  MBB->addSuccessor(Succ, 10)
  MBB->addSuccessor(Succ, 20)

We don't merge these weights, instead getEdgeWeight just returns the first weight added:

  getEdgeWeight(MBB, Succ) -> 10

My patch changes the computed sum from 10+10 to the correct 10+20. This triggered assertions because other code assumes there are no duplicate edges when calling getEdgeWeight().

IMHO, we shouldn't allow duplicate CFG edges at all.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120807/fe4c15f4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: z16.ll
Type: application/octet-stream
Size: 93113 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120807/fe4c15f4/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120807/fe4c15f4/attachment-0001.html>