[PATCH] D129161: [MachinePipeliner] Consider only direct path successors when calculating circuit latency

[Hendrik Greving via Phabricator via llvm-commits]

hgreving added a comment.

Are you working on a backend where writing a test for this is feasible, like Hexagon?

I went through your change and your discourse post and to me this is sound. We're using our own downstream implementation, I am not very familiar with the upstream code. Hoping to find one more reviewer for this.



================
Comment at: llvm/include/llvm/CodeGen/MachinePipeliner.h:339
+      // the first node
+      SUnit *NextSUnit = ((i + 1) == e) ? Nodes[0] : Nodes[i + 1];
+
----------------
This relies on "notes that Johnson's algorithm returns the elementary circuit in the order that the path is taken". Could you add this as a comment here, maybe even add a note at line 321 to SetVector Node, that Node needs to be ordered (and the SetVector isn't there just to avoid non-determinism).

Can you also add a pointer to the fact that it does reutrn the path in order (I myself for example didn't know this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129161