<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 - machine sinking of 2 dest instructions when only one needs BreakPHIEdge"
   href="https://bugs.llvm.org/show_bug.cgi?id=36392">36392</a>
          </td>
        </tr>

        <tr>
          <th>Summary</th>
          <td>machine sinking of 2 dest instructions when only one needs BreakPHIEdge
          </td>
        </tr>

        <tr>
          <th>Product</th>
          <td>new-bugs
          </td>
        </tr>

        <tr>
          <th>Version</th>
          <td>4.0
          </td>
        </tr>

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

        <tr>
          <th>OS</th>
          <td>Linux
          </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>new bugs
          </td>
        </tr>

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

        <tr>
          <th>Reporter</th>
          <td>verena@codeplay.com
          </td>
        </tr>

        <tr>
          <th>CC</th>
          <td>llvm-bugs@lists.llvm.org
          </td>
        </tr></table>
      <p>
        <div>
        <pre>There is a bug in machine sinking of an instruction with two destinations when
exactly one destination needs a PHI edge to be broken.
Unfortunately, I am working on a confidential target, so cannot provide a test
case.

In FindSuccToSinkTo it uses the first destination to set SuccToSinkTo and
BreakPHIEdge, if the PHI edge needs breaking:

for (MachineBasicBlock *SuccBlock : GetAllSortedSuccessors(MI, MBB,
AllSuccessors)) {
  if (AllUsesDominatedByBlock(Reg, SuccBlock, MBB, BreakPHIEdge, LocalUse)) {
    SuccToSinkTo = SuccBlock;

Then, if the instruction has a second destination, it calls
AllUsesDominatedByBlock again

if (SuccToSinkTo) {
   if (!AllUsesDominatedByBlock(Reg, SuccToSinkTo, MBB, BreakPHIEdge,
LocalUse))

But what if the use of the second destination does not need a PHI edge to be
broken? In that case we reset BreakPHIEdge to false. The instruction is sunk
without the PHI edge being broken. This leads to the sunk instruction being
scheduled after the PHI use.
As far as I understand BreakPHIEdge should be true if the uses of *any* of the
destinations need a PHI edge break. Therefore I think the code should be

if (SuccToSinkTo) {
  bool LocalUse = false;
+ bool BreakPHIEdgeOtherDest = false;
  if (!AllUsesDominatedByBlock(Reg, SuccToSinkTo, MBB,
+                              BreakPHIEdgeOtherDest, LocalUse))
    return nullptr;

+ BreakPHIEdge |= BreakPHIEdgeOtherDest;
  continue;
}

Here's a pseudo MIR test for this:

body: |
  bb.0:
    successors: %bb.1, %bb.2

    %1, %2 = TWO_DEST_INSTR
    COND_BRANCH %3, %bb.1
    UNCOND_BRANCH %bb.2

  bb.1:
    successors: %bb.2

    %4 = INSTR

  bb.2:
    successors: %bb.3

    %5 = PHI %1, %bb.0, %4, %bb.1

  bb.3:
    %6 = MOV %2
    RET</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>