[PATCH] D123995: [MachineSink] replace MachineLoop with MachineCycle

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D123995#3461553 <https://reviews.llvm.org/D123995#3461553>, @shchenz wrote:

>> The change description says that MachineSink is "sensitive" to cycle depth. Is it only the performance of the resulting program, or does it affect correctness too?
>
> I think the depth check should only impact the performance in machinesink. However there are some codes(`hasStoreBetween`) which does not work right for irreducible loops, with this patch, we can get correct behavior. But I think we need more investigation for that function. I plan to do more investigation later to avoid errors.

Just to double check, did you verify that hasStoreBetween() works fine with the new implementation? I'm slightly concerned that we may be reintroducing the miscompile depending on how CycleInfo gets chosen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123995