[PATCH] D94308: [MachineSink] SinkIntoLoop: analyse stores and aliases in between

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

Friendly ping. Are we happy with this?

I appreciate that test `X86/loop-sink.mir` is a bit large which I added in the latest revisions, but this has been reduced from a llvm-suite miscompilation and I couldn't reduce it further within a reasonable amount of time. I thought one larger tests wouldn't be bad, actually saw some benefits.
The alternative is that I remove it. The added check is a corner case for X86, and it's an existing check that I am reusing so it is check that is covered. Just for completeness it's this check that I refactored into `HasInstrSideEffects`:

  // If the instruction to move defines a dead physical register which is live
  // when leaving the basic block, don't move it because it could turn into a
  // "zombie" define of that preg. E.g., EFLAGS. (<rdar://problem/8030636>)
  static bool HasInstrSideEffects(MachineInstr &MI, MachineBasicBlock *SinkTo) {


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

https://reviews.llvm.org/D94308