<html>
    <head>
      <base href="https://bugs.llvm.org/">
    </head>
    <body><table border="1" cellspacing="0" cellpadding="8">
        <tr>
          <th>Bug ID</th>
          <td><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - biasCriticalPath does not find the critical path consistently"
   href="https://bugs.llvm.org/show_bug.cgi?id=38689">38689</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>biasCriticalPath does not find the critical path consistently
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>libraries
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>trunk
          </td>
        </tr>

        <tr>
          <th>Hardware</th>
          <td>PC
          </td>
        </tr>

        <tr>
          <th>OS</th>
          <td>All
          </td>
        </tr>

        <tr>
          <th>Status</th>
          <td>NEW
          </td>
        </tr>

        <tr>
          <th>Severity</th>
          <td>normal
          </td>
        </tr>

        <tr>
          <th>Priority</th>
          <td>P
          </td>
        </tr>

        <tr>
          <th>Component</th>
          <td>Common Code Generator Code
          </td>
        </tr>

        <tr>
          <th>Assignee</th>
          <td>unassignedbugs@nondot.org
          </td>
        </tr>

        <tr>
          <th>Reporter</th>
          <td>gongyuwang@micron.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>llvm-bugs@lists.llvm.org
          </td>
        </tr></table>
      <p>
        <div>
        <pre>SUnit::biasCriticalPath() is a simple function that finds the deepest
Predecessor and swap it with the head of this->Preds vector.

Since the code is short, it is probably easier to explain with the code:
void SUnit::biasCriticalPath() {
  if (NumPreds < 2)
    return;

  SUnit::pred_iterator BestI = Preds.begin();
  unsigned MaxDepth = BestI->getSUnit()->getDepth();
  for (SUnit::pred_iterator I = std::next(BestI), E = Preds.end(); I != E;
       ++I) {
    if (I->getKind() == SDep::Data && I->getSUnit()->getDepth() > MaxDepth)
      BestI = I;  // Need to update MaxDepth

  }
  if (BestI != Preds.begin())
    std::swap(*Preds.begin(), *BestI);
}

As the comment in the above code suggest, after BestI is updated, MaxDepth (=
BestI->getSUnit()->getDepth()) should also be updated. But the current code
does not do this, which would fail to find the correct Predecessor
consistently. For example, if this SUnit has three Predecessors (the first
Predecessor has depth 1; the second and third Predecessors are both Data
dependences with depths of 3 and 2 respectively), the above code would fail to
find the second Predecessor as part of the critical path.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are on the CC list for the bug.</li>
      </ul>
    </body>
</html>