This is an archive of the discontinued LLVM Phabricator instance.

ScheduleDAGInstrs: Add condjump deps in addSchedBarrierDeps()
ClosedPublic

Authored by MatzeB on Sep 30 2016, 4:35 PM.

Details

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)

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 73160.Sep 30 2016, 4:35 PM
MatzeB retitled this revision from to ScheduleDAGInstrs: Add condjump deps in addSchedBarrierDeps().
MatzeB updated this object.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.

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

kparzysz edited edge metadata.Oct 3 2016, 11:41 AM

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 added a comment.Oct 3 2016, 4:47 PM

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 updated this revision to Diff 73384.Oct 3 2016, 5:30 PM
MatzeB edited edge metadata.

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.

AMDGPU changes look OK to me.

atrick accepted this revision.Oct 17 2016, 6:33 PM
atrick edited edge metadata.

LGTM.
Thanks!

This revision is now accepted and ready to land.Oct 17 2016, 6:33 PM
This revision was automatically updated to reflect the committed changes.