[PATCH] Fix instruction scheduling live register tracking

Andrew Trick atrick at apple.com
Wed May 27 16:38:23 PDT 2015


Thanks for the fix!

I am generally ok with this even though I still can't prove to myself it's correct, since the existing unit tests (like add-with-overflow-128) still pass and it does fix a nasty bug.

One quick reminder: Make sure you build for all targets before running lit tests.

More importantly, can you provide some explanation for why LiveRegGens cannot have a null entry at the point where you assert.  My understanding is that LiveRegGens is cleared when we schedule the def. So when we're about to unschedule, the entry should be null until we set it again.

The way this is supposed to work (I think):

- While scheduling, assign increasing "heights" to the scheduled nodes.

- When backtracking, we know the successor that should be added back to LiveRegGens is the one with the lowest "height"

This is almost certainly broken because the height field in the DAG node is serving dual purpose, both as a DAG height and a schedule height. One thing to try would using the ScheduleDAG node's BotReadCycle field instead of setting/checking the Height field:

in ScheduleNodeBottom up set BotReadyCycle = CurCycle instead of
SU->setHeightToAtLeast(CurCycle);

Then in backtracking, check BotReadyCycle instead of Height. There may be other bugs in backtracking that you're hitting, but that definitely looks suspect to me.


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9993

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list