[llvm] r298156 - [SelectionDAG] Remove redundant stores more aggressively.

[Eli Friedman via llvm-commits]

Author: efriedma
Date: Fri Mar 17 17:15:50 2017
New Revision: 298156

URL: http://llvm.org/viewvc/llvm-project?rev=298156&view=rev
Log:
[SelectionDAG] Remove redundant stores more aggressively.

Handle TokenFactors more aggressively in
SDValue::reachesChainWithoutSideEffects.  This isn't really a
very effective change anymore because of other changes to
chain handling, but it's a cheap check, and the expanded
comments are still useful.

It might be possible to loosen the hasOneUse() requirement with a
deeper analysis, but a naive implementation of that check would be
expensive.

Differential Revision: https://reviews.llvm.org/D29845



Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
    llvm/trunk/test/CodeGen/ARM/illegal-bitfield-loadstore.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp?rev=298156&r1=298155&r2=298156&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Fri Mar 17 17:15:50 2017
@@ -7317,21 +7317,39 @@ bool SDNode::isOperandOf(const SDNode *N
 /// side-effecting instructions on any chain path.  In practice, this looks
 /// through token factors and non-volatile loads.  In order to remain efficient,
 /// this only looks a couple of nodes in, it does not do an exhaustive search.
+///
+/// Note that we only need to examine chains when we're searching for
+/// side-effects; SelectionDAG requires that all side-effects are represented
+/// by chains, even if another operand would force a specific ordering. This
+/// constraint is necessary to allow transformations like splitting loads.
 bool SDValue::reachesChainWithoutSideEffects(SDValue Dest,
-                                               unsigned Depth) const {
+                                             unsigned Depth) const {
   if (*this == Dest) return true;
 
   // Don't search too deeply, we just want to be able to see through
   // TokenFactor's etc.
   if (Depth == 0) return false;
 
-  // If this is a token factor, all inputs to the TF happen in parallel.  If any
-  // of the operands of the TF does not reach dest, then we cannot do the xform.
+  // If this is a token factor, all inputs to the TF happen in parallel.
   if (getOpcode() == ISD::TokenFactor) {
-    for (unsigned i = 0, e = getNumOperands(); i != e; ++i)
-      if (!getOperand(i).reachesChainWithoutSideEffects(Dest, Depth-1))
-        return false;
-    return true;
+    // First, try a shallow search.
+    if (is_contained((*this)->ops(), Dest)) {
+      // We found the chain we want as an operand of this TokenFactor.
+      // Essentially, we reach the chain without side-effects if we could
+      // serialize the TokenFactor into a simple chain of operations with
+      // Dest as the last operation. This is automatically true if the
+      // chain has one use: there are no other ordering constraints.
+      // If the chain has more than one use, we give up: some other
+      // use of Dest might force a side-effect between Dest and the current
+      // node.
+      if (Dest.hasOneUse())
+        return true;
+    }
+    // Next, try a deep search: check whether every operand of the TokenFactor
+    // reaches Dest.
+    return all_of((*this)->ops(), [=](SDValue Op) {
+      return Op.reachesChainWithoutSideEffects(Dest, Depth - 1);
+    });
   }
 
   // Loads don't have side effects, look through them.

Modified: llvm/trunk/test/CodeGen/ARM/illegal-bitfield-loadstore.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/illegal-bitfield-loadstore.ll?rev=298156&r1=298155&r2=298156&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/illegal-bitfield-loadstore.ll (original)
+++ llvm/trunk/test/CodeGen/ARM/illegal-bitfield-loadstore.ll Fri Mar 17 17:15:50 2017
@@ -159,11 +159,10 @@ define void @i56_insert_bit(i56* %a, i1
 ; BE-NEXT:    .save {r11, lr}
 ; BE-NEXT:    push {r11, lr}
 ; BE-NEXT:    mov r2, r0
+; BE-NEXT:    ldr lr, [r0]
 ; BE-NEXT:    ldrh r12, [r2, #4]!
 ; BE-NEXT:    ldrb r3, [r2, #2]
-; BE-NEXT:    strb r3, [r2, #2]
 ; BE-NEXT:    orr r12, r3, r12, lsl #8
-; BE-NEXT:    ldr lr, [r0]
 ; BE-NEXT:    orr r3, r12, lr, lsl #24
 ; BE-NEXT:    bic r3, r3, #8192
 ; BE-NEXT:    orr r1, r3, r1, lsl #13