[PATCH] D56291: ScheduleDAG: Don't break the dependence in clustering neighboring loads.

[Changpeng Fang via Phabricator via llvm-commits]

cfang marked an inline comment as done.
cfang added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp:258
+    // We continue to search for other candidates.
+    if (Load->isOperandOf(BaseLoad))
+      continue;
----------------
rampitec wrote:
> I see two flaws in this logic:
> 
> 1. A load can depend on any previous load, not just base.
> 2. A dependency can be indirect, not necessarily as a direct operand.
> 
> The problem is if you use a loop over the whole chain and also use isPredecessorOf instead of isOperandOf that will make this loop extremely expensive. It needs to be somehow simplified. Maybe do such check only if getIROrder() of successor is less or equal getIROrder() of a predecessor in the assumption if it was located higher in the IR and given all of that happens in the same basic block, there can be no dependency?
>1.  A load can depend on any previous load, not just base.
Right. But here we should only care about whether the baseload depends on this load because we are trying to create a dependence of the load on the baseload by glueing.

>2. A dependency can be indirect, not necessarily as a direct operand.
llvm::sort(Offsets);
All the leading loads have been checked already, so I am assuming no indirect dependence.  


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

https://reviews.llvm.org/D56291