MachineSink sinking instructions beyond instructions with side effects?

Hello,

While debugging a problem for my out-of-tree backend I noticed that MachineSink doesn’t bother too much about if instructions has side effects. Is this on purpose and is it good to do that?

If does consider side effects of instruction MI when it is analyzing if it is possible to sink MI itself. It does so via MachineInstr::isSafeToMove:

  if (isPosition() || isDebugInstr() || isTerminator() ||
      mayRaiseFPException() || hasUnmodeledSideEffects())
    return false;

But as far as I can see, it does not take into account if the instructions that it may sink MI beyond have side effects. So today it may rewrite

MI
MIWithSideEffect

into

MIWithSideEffect
[...]
MI

I made a quick attempt to prevent MachineSink from sinking over instructions with side effects and got 130 failed lit tests so this seem to happen a lot in tests.

So, is this on purpose and is this in accordance with what hasSideEffects means?