[llvm-dev] Deprecate and remove old SelectionDAG scheduler
James Y Knight via llvm-dev
llvm-dev at lists.llvm.org
Thu Sep 10 15:06:00 PDT 2015
While looking at why some generated code for SPARC was poor, I ran into the fact that the MachineScheduler is not yet enabled by default -- it's opt in for each target. Having a bunch of deprecatedish code around was a bit confusing for newbie me.
So, I suggested on IRC that perhaps it's been long enough by now that any target that hasn't switched over probably isn't actually caring much for the performance of their target. Has the time come to start getting rid of the old SelectionDAG scheduler? I have no pressing *need* to do this, so if the answer is "no", fine. But if it's "yes", I'm willing to do my part for code cleanliness.
(Eric said on IRC that sounded probably a reasonable thing to do but I should ask Andrew first; so cc'd.)
I'd propose to do it in a few steps:
Step 1: Switch defaults, and all in-tree targets away from selectiondag schedulers and to the machine scheduler.
That is, 3 changes:
a) Change enableMachineScheduler() -> true by default, and remove target overrides.
That would change the behavior of the following in-tree targets (resulting in enabling the new scheduler, and implicitly forcing their ScheduleDAG scheduler to Sched::Source):
- ARM (only for some CPUs; others are already using it)
- (Hexagon, only with the non-default -disable-hexagon-misched; I'd remove the option)
- PPC (only for some CPUs; others are already using it)
As part of this, I'd fix tests that fail due to instruction re-orderings, but, I *wouldn't* try to do performance testing of any sort.
The targets from the above list that still use the old code which surprise me are ARM and PPC. However, both also usually seem to return true from enablePostRAScheduler -- maybe they really just want to avoid doing machine scheduling both pre-RA and post-RA, and the fact that it also causes the old selectiondag scheduler to get used is an unintended consequence? (And thus, would actually want enableMachineScheduler() -> false, but with Sched::Source as the selectiondag scheduler?)
b) Change enableMachineSchedDefaultSched() -> true.
Only Hexagon sets that to false currently, with a FIXME near it saying it probably shouldn't. But, that is actually irrelevant, normally, since it uses Sched::Source as its scheduler in any case -- unless an option "-enable-hexagon-sdnode-sched" is passed. I'd remove that option too.
c) Change the default for SchedPreferenceInfo in TargetLoweringBase.cpp to Sched::Source. If everyone also uses enableMachineScheduler() -> true and enableMachineSchedDefaultSched() -> true, this has no effect of course. But just to be clear what the future is, I'd suggest to change it too. :)
After this step, out-of-tree targets could still have "enableMachineScheduler()" return false, and call "setSchedulingPreference(Sched::ILP)" (or whatever other option), if they want to keep using the old scheduler after this step, until step 3 happens.
Step 2: Wait for some time to pass, for people to notice performance regressions, or otherwise complain.
Step 3: Delete all the selectiondag scheduling code, the above 3 customization points, and whatever other hooks/apis are dead. Rejoice at the deletion of code!
Does the above sound sane (today is the first time I've actually looked at all this code, so if I've misunderstood something, do let me know!) Is it okay to execute step 1?
More information about the llvm-dev