[PATCH] D124678: [AMDGPU] Allow for MFMA Inst Clustering

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D124678#3483663 <https://reviews.llvm.org/D124678#3483663>, @jrbyrnes wrote:

> Regarding your note -- yes this is something I spent some time thinking about – Sdep::Cluster doesn’t gaurantee a single cluster. In fact, I believe there is a hardware dependency between MFMA’s, so the scheduler will try to fill this gap with an independent instruction.

This is conflicting thing, we need to make sure it does not succeed to fill the gap. Probably it needs some tweaking in FillMFMAShadowMutation and GCNHazardRecognizer::ShouldPreferAnother if this option is set. In any way you need some more tests with different clustering/non-clustering scenarios and check the final code, do we get resulting clusters? Especially given that post-RA scheduler will try to torn them.

> It seems that if we want to offer unbroken clusters, we will want to offer a scheduler with modified heuristic priorities to prioritize cluster edges. We can also address the issue of UnclusteredReschedule pass in this scheduler. Worth mentioning is that in such a scheme, a cluster can still be broken if the preds / succs of instructions are not handled, which is why I’ve handled them here.

Fair enough. UnclusteredReschedule is also todo for a future patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124678