Skip to content

Commit 5ad7186

Browse files
committed
[DAGCombiner] Improve chain handling in fold (fshl ld1, ld0, c) -> (ld0[ofs]) combine.
Happened to notice some odd things related to chains in this code. I don't have a good test case yet. The code calls hasOneUse on LoadSDNode* which will check users of the data and the chain. I think this was trying to check that the data had one use so one of the loads would definitely be removed by the transform. Load chains don't always have users so our testing may not have noticed that the chains being used would block the transform. The code makes all users of ld1's chain use the new load's chain, but we don't know that ld1 becomes dead. This can cause incorrect dependencies if ld1's chain is used and it isn't deleted. I think the better thing to do is use makeEquivalentMemoryOrdering to make all users of ld0 and ld1 depend on the new load and the original loads. If the olds loads become dead, their chain will be cleaned up later. Please let me know if I've gotten any of this wrong. If it looks good I'll work on getting a test.
1 parent 98d6dd3 commit 5ad7186

File tree

1 file changed

+4
-5
lines changed

1 file changed

+4
-5
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11005,8 +11005,8 @@ SDValue DAGCombiner::visitFunnelShift(SDNode *N) {
1100511005
auto *RHS = dyn_cast<LoadSDNode>(N1);
1100611006
if (LHS && RHS && LHS->isSimple() && RHS->isSimple() &&
1100711007
LHS->getAddressSpace() == RHS->getAddressSpace() &&
11008-
(LHS->hasOneUse() || RHS->hasOneUse()) && ISD::isNON_EXTLoad(RHS) &&
11009-
ISD::isNON_EXTLoad(LHS)) {
11008+
(LHS->hasNUsesOfValue(1, 0) || RHS->hasNUsesOfValue(1, 0)) &&
11009+
ISD::isNON_EXTLoad(RHS) && ISD::isNON_EXTLoad(LHS)) {
1101011010
if (DAG.areNonVolatileConsecutiveLoads(LHS, RHS, BitWidth / 8, 1)) {
1101111011
SDLoc DL(RHS);
1101211012
uint64_t PtrOff =
@@ -11024,9 +11024,8 @@ SDValue DAGCombiner::visitFunnelShift(SDNode *N) {
1102411024
VT, DL, RHS->getChain(), NewPtr,
1102511025
RHS->getPointerInfo().getWithOffset(PtrOff), NewAlign,
1102611026
RHS->getMemOperand()->getFlags(), RHS->getAAInfo());
11027-
// Replace the old load's chain with the new load's chain.
11028-
WorklistRemover DeadNodes(*this);
11029-
DAG.ReplaceAllUsesOfValueWith(N1.getValue(1), Load.getValue(1));
11027+
DAG.makeEquivalentMemoryOrdering(LHS, Load.getValue(1));
11028+
DAG.makeEquivalentMemoryOrdering(RHS, Load.getValue(1));
1103011029
return Load;
1103111030
}
1103211031
}

0 commit comments

Comments
 (0)