From: Eli Friedman Date: Fri, 17 Mar 2017 22:15:50 +0000 (+0000) Subject: [SelectionDAG] Remove redundant stores more aggressively. X-Git-Tag: android-x86-7.1-r4~18808 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=87552d6290872a325d51f6137a6121d145ff5ec3;p=android-x86%2Fexternal-llvm.git [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 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@298156 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp index 90987d38744..d45898ea757 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAG.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAG.cpp @@ -7317,21 +7317,39 @@ bool SDNode::isOperandOf(const SDNode *N) const { /// 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. diff --git a/test/CodeGen/ARM/illegal-bitfield-loadstore.ll b/test/CodeGen/ARM/illegal-bitfield-loadstore.ll index bfb2b95c256..74117d3896b 100644 --- a/test/CodeGen/ARM/illegal-bitfield-loadstore.ll +++ b/test/CodeGen/ARM/illegal-bitfield-loadstore.ll @@ -159,11 +159,10 @@ define void @i56_insert_bit(i56* %a, i1 zeroext %bit) { ; 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