[PATCH] D9067: LiveIntervalAnalysis: Support moving of subregister defs in handleMove

[Axel Davy via llvm-commits]

axeldavy added a comment.

In http://reviews.llvm.org/D9067#324263, @MatzeB wrote:

> I didn't read all the code in that patch. As long as you respect all the dependencies generated by ScheduleDAGInstructions today you should not run into any situations where handleMove fails,  The changes here should only be necessary if subregister definitions become independent of each other. If you still require this patch then we may have an unrelated bug in handleMove(). Would be interesting to see the liveinterval before and for a handleMove() that fails for you.


The ordering constraints of ScheduleDAGInstructions are definitely preserved.

The scheduler does make blocks of instructions, and does a first schedule such that instructions inside a block are all consecutive (handleMove is used first there).
Then the blocks order is changed, and that results in second handleMove for the final schedule.

The first problem I face is that if before the second handleMove I don't restore the original ordering, then the second handleMove will crash.
With or without your patch, it is needed.

Step 1) Order instructions by blocks
Step 1 bis) Restore old ordering
Step 2) Put final instruction order

That works with and without your patch (bis step required, else it crashes)

The second problem I face is that as improvement to the scheduler, I want it to try other strategies if the previous result was not satisfying enough.
So this becomes:
Step 1) Order instructions by blocks
Step 1 bis) Restore old ordering
Step 2) Order instructions by blocks (different blocks)
Step 2 bis) Restore old ordering
Step 3) Put final instruction order

Without your patch, Step 2 crashes. Your patch makes the whole work.
The bis steps are still required.


Repository:
  rL LLVM

http://reviews.llvm.org/D9067