[PATCH] D41293: [DAG, X86] Improve Dependency analysis when doing multi-node Instruction Selection

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/SelectionDAGNodes.h:804
+                                   unsigned int MaxSteps = 0,
+                                   bool TopologicalPrune = false) {
+    SmallVector<const SDNode *, 16> DeferredNodes;
----------------
Should document what the new argument is for in function comment.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2257
     // the user (which has already been selected) has a chain or indirectly uses
     // the chain, our WalkChainUsers predicate will not consider it.  Because of
     // this, we cannot ignore chains in this predicate.
----------------
Still mentions WalkChainUsers, but that function is deleted.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2439
+
+  if (InputChains.size() == 0)
+    return CurDAG->getEntryNode();
----------------
This can move up immediately after the call to AddChains, right?


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2111
+
+    bool ChainCheck = false;
     SmallVector<SDValue, 4> ChainOps;
----------------
Rename ChainCheck "FoundLoad".


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2128
+    // If Loop Worklist is not empty. Check if we would make a loop.
+    if (ChainCheck) {
+      if (!LoopWorklist.empty()) {
----------------
if (!FoundLoad) return false; and unindent the rest.


================
Comment at: llvm/test/CodeGen/X86/2012-01-16-mfence-nosse-flags.ll:17
 ; CHECK: lock orl {{.*}}, (%esp)
-; CHECK-NEXT: testl [[REG:%e[a-z]+]], [[REG]]
+; CHECK-NEXT: cmpl $0, [[REG:%e[a-z]+]]
 
----------------
niravd wrote:
> craig.topper wrote:
> > Why did we fail to use testl here now?
> ISel now can merge the load with the compare operation. We later split it in a call to unfoldMemoryOperand but we only do the check  to covert compares back to tests in the MI verstion not the DAG version that's being called.  
> 
> 
> 
Put that into a comment maybe?


Repository:
  rL LLVM

https://reviews.llvm.org/D41293