[PATCH] D53485: [ScheduleDAGRRList] Do not preschedule the node has ADJCALLSTACKDOWN parent

Shiva Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 3 02:31:56 PDT 2018


shiva0217 added a comment.

Hi @samparker,
Thanks for your time to take a look at the code.

Hi @eli.friedman,
Thanks for the comments.
Yes, that's exactly how the issue happened.
It seems that PrescheduleNodesWithMultipleUses() try to pre-schedule the store which is one of the multiple users, because register pressure heuristic may increase the live range of other use.
But when the dependency is not the data dependence,  the register pressure won't increase because there is no real register using between N and U in the following case.
We probably could avoid to pre-schedule the node when it's not data dependency. Does it sound reasonable or it could drop the performance in some cases?

      N
    / |
   /  |
  U  store

I try to reproduce the fail case by in-tree targets, but it seems that it would depend on ISA and scheduling model and no in-tree targets could trigger the issue.

Hi @TimNN,
Is your test case be able to trigger the issue by in-tree targets?

I'm sorry that I'll take a few days off because my first child going to born, but I'll catch up as soon as I can.


Repository:
  rL LLVM

https://reviews.llvm.org/D53485





More information about the llvm-commits mailing list