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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2135
 
-/// findNonImmUse - Return true if "Use" is a non-immediate use of "Def".
-/// This function iteratively traverses up the operand chain, ignoring
-/// certain nodes.
-static bool findNonImmUse(SDNode *Use, SDNode* Def, SDNode *ImmedUse,
-                          SDNode *Root, SmallPtrSetImpl<SDNode*> &Visited,
+/// findNonImmUse - Return true if "Use" is a non-immediate use of
+/// "Def".  This function iteratively traverses up all operands. We
----------------
There's no longer a parameter named "Use" so the description doesn't make sense anymore


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2399
+    } else if (V->getOpcode() == ISD::TokenFactor)
+      for (int i = 0, e = V->getNumOperands(); i != e; ++i)
+        AddChains(V->getOperand(i));
----------------
Add curly braces around this body for readability's sake


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:2416
+  // to-be-merged nodes. Fail.
+    Visited.clear();
+  for (SDValue V : InputChains)
----------------
I think this Visited.clear call is indented incorrectly?


================
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]+]]
 
----------------
Why did we fail to use testl here now?


================
Comment at: llvm/test/CodeGen/X86/subvector-broadcast.ll:922
+; X64-AVX1-NEXT:    vxorps %xmm1, %xmm1, %xmm1
+; X64-AVX1-NEXT:    vbroadcastf128 {{.*#+}} ymm0 = mem[0,1,0,1]
+; X64-AVX1-NEXT:    vmovaps %xmm1, (%rsi)
----------------
There's seems to be a bug in the execution domain vbroadcastf128 where its being reported as Int instead of FP which is causing the vmovaps/vmovdq difference between AVX1 and AVX2. I'm going to fix that so you'll probably need to rebase this area.


https://reviews.llvm.org/D41293