[PATCH] D86925: [MachineSink] add one more profitable pattern for sinking

[ChenZheng via Phabricator via llvm-commits]

shchenz added a comment.

@qcolombet Thanks for your good comments.

> given we are looking at loop invariant here, it would make sense to have this code in MachineLICM instead of MachineSink

MachineLICM tries to move loop invariant instructions to outside of a loop(preheader of exit block). Here we want to move def closer to its use inside the loop. 
If we want to support 'real' sink in MachineLICM(move loop invariant instructions to the exit block), it should always benefit for the loop's register pressure?

If we want to move def closer to its use inside the loop to benefit register pressure in MachineLICM pass, except many duplicated codes with machinesink pass, (condition checks in `SinkInstruction`), register estimate model in machineLICM also does not support this. I checked `UpdateBackTraceRegPressure`, seems now the model is designed for hoisting only. It just stores the register pressure on path from loop preheader to current BB. see `RegPressure` and `calcRegisterCost`(Sinking should have a different cost calculation). For sinking, we need to estimate the register pressure in the destination block which is not on path from loop preheader to current BB.

The real world case I met is not a loop invariant instruction, it has an operand which is a user of PHI in loop header.

So I think maybe we should implement this in machinesink pass?

But yes, the register pressure estimate is not accurate for now and your example shows this very well. If we want to estimate it accurately, maybe we can use `struct RegisterPressure` in `RegisterPressure.h` to check register pressure of the destinate BB after the sinking? If the register pressure for register pressure set exceeds the limit after the sinking, we don't sink. What do you think? @qcolombet

Thanks again for your comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86925/new/

https://reviews.llvm.org/D86925