[llvm] [MachinePipeliner] Fix incorrect handlings of unpipelineable insts (PR #126057)

[Ryotaro Kasuga via llvm-commits]

kasuga-fj wrote:

> There's a bit of confusion of terminology around dependence chai

Sorry for the confusion, yes, the term "dependency chain" seems to usually refer to barrier dependencies. What I meant was  what you wrote.

> So, I think what's happened here is that the changes to the data dependence handling has probably confused the issue

I'm not sure if I understand you correctly, but if "the changes to the data dependence handling" refers to #109918, I think this issue is not related to it. I've confirmed that the same problem happens in older versions of LLVM (before that patch was merged).

To answer your questions:

> Is it possible that there was code removed from normalizeNonPipelinedInstructions which formerly ignored the artificial dependence when computing SU(14)'s scheduling time?

No, I think.

> Was the anti-dependence traversed backwards previously?

Yes, it's been traversed backwards from the beginning.

https://github.com/llvm/llvm-project/blob/dcb77643e3440e948010ed8ecb4c2f8fe4fadb93/llvm/lib/CodeGen/MachinePipeliner.cpp#L2729-L2732

> Is the artificial dependence correct?

I think it is correct, because swapping the source and destination instruction doesn't change the original semantics. However, it does create the inconsistency with SU order. This artificial dependence was created by `CopyToPhiMutation`. I don't fully understand what it does, but it seems to be some heuristic to shorten the live-range of some variables. And it adds artificial dependencies to not break the topological order: not SU order.


In conclusion, I agree with you on the following.

> executing in SU order should be correct -- no actual dependence can be violated

However, an artificial dependence is not an actual dependence. Ignoring all artificial dependencies in normalizeNonPipelinedInstructions may be a solution, but I don't know if it is correct in all cases.

https://github.com/llvm/llvm-project/pull/126057