[llvm] [MachinePipeliner] Add an abstract layer to manipulate Data Dependenc… (PR #109918)

Ryotaro Kasuga via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 23 02:46:09 PDT 2024


kasuga-fj wrote:

Thanks for your detailed comments!

>  Is that a problem in general? I'd have to think more on it.

Unless we can determine that it's not going to be a problem, I think we should be conservative with dependency analysis. However, as you said, it's also true that I don't have an example that triggers invalid code generation. It also complicates things that missing dependencies do not necessarily cause invalid code to be generated.

In any case, the FIXME comments I added in this patch need to be changed.

>  The only question is whether the removal of the reversal code around createAdjacencyStructure is going to create a new failure case in the presence of loop-carried physical register or memory dependencies. That removal results in fewer computed circuits. Circuits are used for MII calculation and for checkValidNodeOrder. MII calculation is not a correctness issue. As for validity, being in a circuit makes it legal to be after both successors and predecessors, so fewer circuits means fewer legal orders, so I don't think we can introduce new failure cases, just reduced opportunities for pipelining.

I believe that the circuits to be detected do not change with or without this patch, because
  - If I understand correctly, all memory dependencies are represented as order-dependence, so they are not affected by `swapAntiDependences`.
  - In the original implementation, anti-dependencies originated from physical registers are completely ignored.

Also, `checkValidNodeOrder` does not prevent loops from being scheduled even if they have an invalid node order. It just checks the order, but doesn't inhibit subsequent processes. 

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


More information about the llvm-commits mailing list