[llvm-bugs] [Bug 36392] New: machine sinking of 2 dest instructions when only one needs BreakPHIEdge

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=36392

            Bug ID: 36392
           Summary: machine sinking of 2 dest instructions when only one
                    needs BreakPHIEdge
           Product: new-bugs
           Version: 4.0
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: verena at codeplay.com
                CC: llvm-bugs at lists.llvm.org

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

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20180215/3241b164/attachment.html>