Added operand description to TargetInstrInfo::shouldClusterMemOps()

Changed patch to only correct clusterNeighboringMemOps. Threshold logic adjusted accordingly.
We will need to retune our threshold in the AMDGPU separately, likely after scheduler will gain the ability to break clustering under a high pressure.

LGTM

LGTM

LGTM

LGTM

LGTM

LGTM. Thanks.

LGTM

LGTM as long as parent is submitted.

In D73292#1838140, @foad wrote:

In D73292#1837231, @rampitec wrote:

In D73292#1837215, @foad wrote:

I tried something similar in D72325.

Comments there argue about how much should we cluster, but regardless I do not think we should use a wrong data. If we want more clustering we need to increase thresholds, but still rely on a correct input.

I agree. I also think we should fix this properly in MachineScheduler:

--- a/llvm/lib/CodeGen/MachineScheduler.cpp
+++ b/llvm/lib/CodeGen/MachineScheduler.cpp
@@ -1584,7 +1584,7 @@ void BaseMemOpClusterMutation::clusterNeighboringMemOps(
     SUnit *SUb = MemOpRecords[Idx+1].SU;
     if (TII->shouldClusterMemOps(MemOpRecords[Idx].BaseOps,
                                  MemOpRecords[Idx + 1].BaseOps,
-                                 ClusterLength)) {
+                                 ClusterLength + 1)) {
       if (SUa->NodeNum > SUb->NodeNum)
         std::swap(SUa, SUb);
       if (DAG->addEdge(SUb, SDep(SUa, SDep::Cluster))) {

... and adjust any other target implementations of shouldClusterMemOps accordingly.

What has caused the failure? Will it happen on Windows in real life?

I also have an early version of a patch which can reschedule without clustering if no optimal schedule were found for a region.

In D73292#1837215, @foad wrote:

I tried something similar in D72325.

LGTM

Pre-commited NFC changes and rebased.

LGTM

LGTM

LGTM. A verifier update is desirable though.

LGTM

LGTM

The change seems to miss parent revision introducing V_INDIRECT_REG_WRITE_*.

LGTM

LGTM

LGTM

Prevent bundling of loads with overlapping destinations. They are dependent.

LGTM

LGTM

LGTM

I think we need to add FeatureR128A16 to GFX10. ISA spec says it does support it.

Added subtarget predicate.

LGTM

LGTM

LGTM.

Rebased on top of D72783 to prevent tail duplication of a bundle.

In D72737#1821378, @foad wrote:

There are nice changes in a bunch of tests, where we're preserving clusters instead of breaking them apart.

But there are also strange changes in some other tests, where the clustering hasn't changed, but some instructions that use the result of a load have moved around. Does this mean we're getting the latency of the load wrong now? (Or were we getting it wrong before?) For example:
insert_vector_elt
llvm.maxnum.f16.ll
saddo.ll
sign_extend.ll

In D72737#1820974, @kerbowa wrote:

In D72737#1820946, @rampitec wrote:

In D72737#1820865, @kerbowa wrote:

By the way I think we actually do try to insert waitcnt into bundles. I wonder if it would be worth it to just update the memory legalizer to work with bundles as well.

I saw waitcount inserted after the bundle if I do not unpack them, so probably we do not do it always if we really do.

Yeah maybe. I remember it was added here: c04aab9c0646461bc187808920b3d5ee7f5cc5ab
Was the waitcnt after the bundle that you saw in the correct place?

Rebased.

Fixed bug in the bundling logic and added test for produced bundles and when do they break.

In D72737#1820865, @kerbowa wrote:

By the way I think we actually do try to insert waitcnt into bundles. I wonder if it would be worth it to just update the memory legalizer to work with bundles as well.

In D72737#1820905, @kerbowa wrote:

Actually I misunderstood the change because of the title. I see now that the loads are bundled before post-RA scheduler.

I have found a bug in the logic. Will update the review.

In D72737#1820814, @arsenm wrote:

Is this just working around the post-RA scheduler? Can we finally replace the post-RA scheduler with misched?

LGTM

You have skipped the dead MO, but was pass reordering really necessary? It seems we have higher register pressure with this change.

LGTM