ScheduleDAGInstrs: Add condjump deps in addSchedBarrierDeps()

[MatzeB]

Summary

addSchedBarrierDeps() is supposed to add use operands to the ExitSU
node. The current implementation adds uses for calls/barrier instruction
and the MBB live-outs in all other cases. The use
operands of conditional jump instructions were missed.

(I'd also find adding the live-outs of the basic block questionable as I would assume it to be more likely a value is not used immediately, but we can have that discussion another day)

[MatzeB]

Note that this is a review to get some feedback on the change, testcase adjustments are still outstanding.

[kparzysz]

Looks ok to me.

Should def operands in branches also be treated specially (e.g. adding a dependency with a successor's live-in)?

[MatzeB]

In D25140#559480, @kparzysz wrote:

Should def operands in branches also be treated specially (e.g. adding a dependency with a successor's live-in)?

It shouldn't matter for scheduling: Branch instructions are only allowed at the end of a basic block, so by definition we cannot have any non-branch instructions between them, so we will never get any interesting scheduling region formed between branches.

[MatzeB]

New version:

• Adjust test cases to changed schedule
• Now that we have proper dependency on the conditional branches we can simplify the macro fusion code to only check the ExitSU predecessors and we can also skip the dependency check there.

[• tstellarAMD]

AMDGPU changes look OK to me.

[MatzeB]

ping

[atrick]

LGTM.
Thanks!

lib/CodeGen/ScheduleDAGInstrs.cpp

 /// make sure instructions which define registers that are either used by
 /// the terminator or are live-out are properly scheduled. This is
 /// especially important when the definition latency of the return value(s)
 /// are too high to be hidden by the branch or when the liveout registers
 /// used by instructions in the fallthrough block.
 void ScheduleDAGInstrs::addSchedBarrierDeps() {
   MachineInstr *ExitMI = RegionEnd != BB->end() ? &*RegionEnd : nullptr;
   ExitSU.setInstr(ExitMI);
-  bool AllDepKnown = ExitMI &&
-    (ExitMI->isCall() || ExitMI->isBarrier());
-  if (ExitMI && AllDepKnown) {
-    // If it's a call or a barrier, add dependencies on the defs and uses of
-    // instruction.
+  // Add dependencies on the defs and uses of the instruction.
+  if (ExitMI) {
     for (const MachineOperand &MO : ExitMI->operands()) {
       if (!MO.isReg() || MO.isDef()) continue;
       unsigned Reg = MO.getReg();
       if (Reg == 0) continue;
 
       if (TRI->isPhysicalRegister(Reg))
         Uses.insert(PhysRegSUOper(&ExitSU, -1, Reg));
       else if (MO.readsReg()) // ignore undef operands
         addVRegUseDeps(&ExitSU, ExitMI->getOperandNo(&MO));
     }
-  } else {
+  }
+  if (!ExitMI || (!ExitMI->isCall() && !ExitMI->isBarrier())) {
     // For others, e.g. fallthrough, conditional branch, assume the exit
     // uses all the registers that are livein to the successor blocks.
-    assert(Uses.empty() && "Uses in set before adding deps?");
     for (const MachineBasicBlock *Succ : BB->successors()) {
       for (const auto &LI : Succ->liveins()) {
         if (!Uses.contains(LI.PhysReg))
           Uses.insert(PhysRegSUOper(&ExitSU, -1, LI.PhysReg));
       }
     }
   }
 }