[PATCH] D58070: [DAG] Remember nodes added to the worklist for pruning.

[James Y Knight via Phabricator via llvm-commits]

jyknight added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:140
     DenseMap<SDNode *, unsigned> WorklistMap;
+    SmallSetVector<SDNode *, 32> PruningList;
 
----------------
Couldn't this just be represented by an index of the last position in the worklist which has been examined for new-node-prunning, instead of a separate list?

Set it to Worklist.size() after popping entries in getNextWorklistEntry(), and iterate from it to the end of worklist in clearNewUnusedWorklistEntries.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:160
+      // Check any nodes added to the worklist to see if they are prunable.
+      SmallSetVector<SDNode *, 16> Nodes;
+
----------------
Unused?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:205
 
+    void AddNewNode(SDNode *N) { PruningList.insert(N); }
+
----------------
Unused?


================
Comment at: llvm/test/CodeGen/X86/constant-combines.ll:12-14
 ; it folded it to a zero too late to legalize the zero store operation. If this
 ; ever starts forming a zero store instead of movss, the test case has stopped
 ; being useful.
----------------
The comment here says this test is now useless.


================
Comment at: llvm/test/CodeGen/X86/pr28504.ll:9
 ; Basically, what we want to see is that the compare result zero-extended, and 
 ; then stored. Only one zext, and no sexts.
 
----------------
Given this comment, I wonder if this test case is still valid?


================
Comment at: llvm/test/CodeGen/X86/pr33844.ll:13
 ; CHECK:       # %bb.0: # %bb
-; CHECK-NEXT:    movl {{.*}}(%rip), %eax
-; CHECK-NEXT:    movl %eax, %ecx
-; CHECK-NEXT:    shrl $31, %ecx
-; CHECK-NEXT:    addl $2147483647, %ecx # imm = 0x7FFFFFFF
-; CHECK-NEXT:    shrl $31, %ecx
-; CHECK-NEXT:    andl $-2, %ecx
-; CHECK-NEXT:    andl $-536870912, %eax # imm = 0xE0000000
-; CHECK-NEXT:    orl %ecx, %eax
-; CHECK-NEXT:    movl %eax, {{.*}}(%rip)
+; CHECK-NEXT:    andl $-536870912, {{.*}}(%rip) # imm = 0xE0000000
 ; CHECK-NEXT:    retq
----------------
This looks like it might not be testing what it intended to test anymore, too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58070