Skip to content

Commit f693932

Browse files
committed
[SelectionDAG] Transitively copy NodeExtraInfo on RAUW
During legalization of the SelectionDAG, some nodes are replaced with arch-specific nodes. These may be complex nodes, where the root node no longer corresponds to the node that should carry the extra info. Fix the issue by copying extra info to the new node and all its new transitive operands during RAUW. See code comments for more details. This fixes the remaining pcsections-atomics.ll tests on X86. v2: Optimize copyExtraInfo() deep copy. For now we assume that only NodeExtraInfo that have PCSections set require deep copy. Furthermore, limit the depth of graph search while pre-populating the visited set, assuming the to-be-replaced subgraph 'From' has limited complexity. An assertion catches if the maximum depth needs to be increased. Reviewed By: dvyukov Differential Revision: https://reviews.llvm.org/D144677
1 parent 057b6fb commit f693932

File tree

3 files changed

+1382
-877
lines changed

3 files changed

+1382
-877
lines changed

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "llvm/ADT/APSInt.h"
1818
#include "llvm/ADT/ArrayRef.h"
1919
#include "llvm/ADT/BitVector.h"
20+
#include "llvm/ADT/DenseSet.h"
2021
#include "llvm/ADT/FoldingSet.h"
2122
#include "llvm/ADT/STLExtras.h"
2223
#include "llvm/ADT/SmallPtrSet.h"
@@ -12223,7 +12224,56 @@ void SelectionDAG::copyExtraInfo(SDNode *From, SDNode *To) {
1222312224
// Use of operator[] on the DenseMap may cause an insertion, which invalidates
1222412225
// the iterator, hence the need to make a copy to prevent a use-after-free.
1222512226
NodeExtraInfo Copy = I->second;
12226-
SDEI[To] = std::move(Copy);
12227+
if (LLVM_LIKELY(!Copy.PCSections)) {
12228+
// No deep copy required for the types of extra info set.
12229+
SDEI[To] = std::move(Copy);
12230+
return;
12231+
}
12232+
12233+
// We need to copy NodeExtraInfo to all _new_ nodes that are being introduced
12234+
// through the replacement of From with To. Otherwise, replacements of a node
12235+
// (From) with more complex nodes (To and its operands) may result in lost
12236+
// extra info where the root node (To) is insignificant in further propagating
12237+
// and using extra info when further lowering to MIR.
12238+
//
12239+
// In the first step pre-populate the visited set with the nodes reachable
12240+
// from the old From node. This avoids copying NodeExtraInfo to parts of the
12241+
// DAG that is not new and should be left untouched.
12242+
DenseSet<const SDNode *> Visited;
12243+
constexpr int MaxDepth = 16;
12244+
auto VisitFrom = [&Visited](auto &&Self, SDNode *N, int Depth) {
12245+
if (Depth >= MaxDepth)
12246+
return;
12247+
if (!Visited.insert(N).second)
12248+
return;
12249+
for (const SDValue &Op : N->op_values())
12250+
Self(Self, Op.getNode(), Depth + 1);
12251+
};
12252+
VisitFrom(VisitFrom, From, 0);
12253+
12254+
// Copy extra info to To and all its transitive operands (that are new).
12255+
auto DeepCopyTo = [this, &Copy, &Visited](auto &&Self, SDNode *To) {
12256+
if (!Visited.insert(To).second)
12257+
return true;
12258+
if (getEntryNode().getNode() == To) {
12259+
// This should not happen - and if it did, that means From has a depth
12260+
// greater or equal to MaxDepth, and VisitFrom() could not visit all
12261+
// common operands. As a result, we're able to reach the entry node.
12262+
assert(false && "Too complex 'From' node - increase MaxDepth?");
12263+
return false;
12264+
}
12265+
for (const SDValue &Op : To->op_values()) {
12266+
if (!Self(Self, Op.getNode()))
12267+
return false;
12268+
}
12269+
SDEI[To] = Copy;
12270+
return true;
12271+
};
12272+
12273+
if (LLVM_UNLIKELY(!DeepCopyTo(DeepCopyTo, To))) {
12274+
// Fallback - see assert above.
12275+
SDEI[To] = std::move(Copy);
12276+
}
1222712277
}
1222812278

1222912279
#ifndef NDEBUG

llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,12 @@ void SDNode::print_details(raw_ostream &OS, const SelectionDAG *G) const {
851851
Dbg->print(OS);
852852
} else if (getHasDebugValue())
853853
OS << " [NoOfDbgValues>0]";
854+
855+
if (const auto *MD = G ? G->getPCSections(this) : nullptr) {
856+
OS << " [pcsections ";
857+
MD->printAsOperand(OS, G->getMachineFunction().getFunction().getParent());
858+
OS << ']';
859+
}
854860
}
855861
}
856862

0 commit comments

Comments
 (0)