Page MenuHomePhabricator

[NFC] Add the infrastructure to forward the scheduled state between MBB
Needs ReviewPublic

Authored by steven.zhang on Apr 28 2019, 7:31 PM.

Details

Summary

Forward the scheduled state from the single pred MBB, as they are belong to the the same window from pipeline view.

Diff Detail

Event Timeline

steven.zhang created this revision.Apr 28 2019, 7:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2019, 7:31 PM
lei added a subscriber: lei.Wed, Jun 12, 5:16 AM
lei added inline comments.
llvm/include/llvm/CodeGen/MachineScheduler.h
233

Do you mean Return true if scheduled state is saved successfully before leaving the MBB?

239

Does pred mean predecessor? I think it is best to use the full word for code documentation.

239

Maybe this is more clear?

/// Return predecessor if MBB have only a single predecessor.
fhahn added inline comments.Mon, Jun 17, 3:28 AM
llvm/include/llvm/CodeGen/MachineScheduler.h
312–313

IIUC this would drop the way to specify this via the scheduling policy and could change behavior for existing policies around.

It might make sense to have state forwarding imply top down here (SchedImpl->doMBBSchedRegionsTopDown() || MF.getSubtarget().forwardScheduledState())

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
197 ↗(On Diff #197051)

Is there a benefit of putting this setting in here, rather than in the scheduling policy, like similar settings? Unless I am missing something, I think it would be more suitable in the scheduling policy, where we already have similar options.

steven.zhang marked 4 inline comments as done.

Address reviewer's comments. Thank you!

llvm/include/llvm/CodeGen/MachineScheduler.h
239

Thank you for the comments. I will update it.

239

Yes, it is. I will use the full item.

312–313

ok.

llvm/include/llvm/CodeGen/TargetSubtargetInfo.h
197 ↗(On Diff #197051)

Sounds good.