This is an archive of the discontinued LLVM Phabricator instance.

MachineScheduler: Allow independent scheduling of sub register defs
ClosedPublic

Authored by MatzeB on Nov 24 2015, 2:13 PM.

Details

Summary

If the TrackLaneMasks policy/strategy is enabled the MachineScheduler
will build a schedule graph where definitions of independent
subregisters are no longer serialised.

Implementation comments:

  • Without lane mask tracking a sub register def also counts as a use (except for the first one with the read-undef flag set), with lane mask tracking enabled this is no longer the case.
  • Pressure Diffs where previously maintained per definition of a vreg with the help of the SSA information contained in the LiveIntervals. With lanemask tracking enabled we cannot do this anymore and instead change the pressure diffs for all uses of the vreg as it becomes live/dead. For this changed style to work correctly we ignore uses of instructions that define the same register again: They won't affect register pressure.
  • With lanemask tracking we remove all read-undef flags from sub register defs when building the graph and re-add them later when all vreg lanes have become dead.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 41086.Nov 24 2015, 2:13 PM
MatzeB retitled this revision from to MachineScheduler: Allow independent scheduling of sub register defs.
MatzeB updated this object.
MatzeB added a reviewer: atrick.
MatzeB set the repository for this revision to rL LLVM.
atrick requested changes to this revision.Nov 30 2015, 10:49 AM
atrick edited edge metadata.

How does this avoid creating multiple disjoint live subranges for a single virtual register? Will verifyLiveInterval choke on this?

RegisterPressureTracker now mutates the machine instructions which is conceptually wrong. If there is no other efficient way to handle setting the read-undef flags, then this needs to be very clearly documented in the tracker API and where the scheduler forces it to be used for subregisters.

lib/CodeGen/MachineScheduler.cpp
335–337 ↗(On Diff #41086)

How are multiple disjoint live segments avoided in general for non-dead sub register definitions? What if the sub register definitions initially overlap and scheduling makes them disjoint? It looks like we place a read-undef flag on subsequent definitions creating multiple "connected components" in the live range.

This revision now requires changes to proceed.Nov 30 2015, 10:49 AM

How does this avoid creating multiple disjoint live subranges for a single virtual register? Will verifyLiveInterval choke on this?

RegisterPressureTracker now mutates the machine instructions which is conceptually wrong. If there is no other efficient way to handle setting the read-undef flags, then this needs to be very clearly documented in the tracker API and where the scheduler forces it to be used for subregisters.

Yes the current code feels very wrong in this respect. To properly fix this I have to go into RegisterPressure and expose the RegisterOperands class publicly so the RegisterOperands can be built in the MachineScheduler and then be passed on to RegisterPressure and used to set the flags.
As that is a bigger refactoring I tried to avoid it in the context of this patch, but I can understand that you do not want these layering violations in the code, I'll look into the refactoring and propose separate patches for that.

lib/CodeGen/MachineScheduler.cpp
335–337 ↗(On Diff #41086)

You are right, this cannot only happen for dead-subreg defs but may also happen for degenerated cases where we have completely independent def+use pairs for subregisters. I think except for completely dead subregister defs this won't happen in typical programs since the whole point of using subregisters is to have some defs or uses where they are joined.

But we certainly need MachineVerifier checking for this property. I'll work on that.

MatzeB updated this revision to Diff 42745.Dec 14 2015, 11:41 AM
MatzeB edited edge metadata.

New versions which builds RegisterOperands in MachineScheduler instead of RegisterPressure. This has the effect that the machine instructions are modified outside of the RegisterPressureTracker now.

atrick accepted this revision.Dec 26 2015, 7:18 PM
atrick edited edge metadata.

LGTM as long as you can verify that subreg lifetimes are not disjoint.

This revision is now accepted and ready to land.Dec 26 2015, 7:18 PM
This revision was automatically updated to reflect the committed changes.