diff --git a/lib/DataLayoutAnalysis/DLAPass.cpp b/lib/DataLayoutAnalysis/DLAPass.cpp index 7c1cf3de4..213d22b4c 100644 --- a/lib/DataLayoutAnalysis/DLAPass.cpp +++ b/lib/DataLayoutAnalysis/DLAPass.cpp @@ -56,6 +56,7 @@ bool DLAPass::runOnModule(llvm::Module &M) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); diff --git a/lib/DataLayoutAnalysis/DLATypeSystem.cpp b/lib/DataLayoutAnalysis/DLATypeSystem.cpp index ad0dbdc71..aa7a1bb0c 100644 --- a/lib/DataLayoutAnalysis/DLATypeSystem.cpp +++ b/lib/DataLayoutAnalysis/DLATypeSystem.cpp @@ -446,10 +446,17 @@ bool LayoutTypeSystem::verifyConsistency() const { unsigned NonPtrChildren = 0U; bool IsPointer = false; for (const auto &Edge : NodePtr->Successors) { - if (isPointerEdge(Edge)) + if (isPointerEdge(Edge)) { + // Node with outgoing pointer edges, only have one pointer edge. + if (IsPointer) { + if (VerifyDLALog.isEnabled()) + revng_check(false); + return false; + } IsPointer = true; - else if (isInstanceEdge(Edge)) + } else if (isInstanceEdge(Edge)) { NonPtrChildren++; + } if (IsPointer and NonPtrChildren > 0) { if (VerifyDLALog.isEnabled()) diff --git a/lib/DataLayoutAnalysis/Middleend/DLACollapseSingleChild.cpp b/lib/DataLayoutAnalysis/Middleend/DLACollapseSingleChild.cpp index 6325ceaeb..c6599601d 100644 --- a/lib/DataLayoutAnalysis/Middleend/DLACollapseSingleChild.cpp +++ b/lib/DataLayoutAnalysis/Middleend/DLACollapseSingleChild.cpp @@ -24,51 +24,44 @@ static Logger<> Log("dla-collapse-single-child"); namespace dla { bool CollapseSingleChild::collapseSingle(LayoutTypeSystem &TS, LayoutTypeSystemNode *Node) { + LoggerIndent Indent{ Log }; bool Changed = false; - auto HasSingleChild = [](const LTSN *Node) { - return (Node->Successors.size() == 1); + auto HasSingleChildAtOffset0 = [](const LTSN *Node) { + return (Node->Successors.size() == 1) + and isInstanceOff0(*Node->Successors.begin()); }; - auto HasAtMostOneParent = [](const LTSN *Node) { - return (Node->Predecessors.size() <= 1); - }; - - if (VerifyLog.isEnabled() and not HasSingleChild(Node)) { - revng_assert(llvm::none_of(Node->Successors, - [](const Link &L) { return isPointerEdge(L); })); - } - - // Get nodes that have a single instance or inheritance child - if (HasSingleChild(Node) and isInstanceEdge(*Node->Successors.begin())) { + // Get nodes that have a single instance-at-offset-0 child + while (HasSingleChildAtOffset0(Node)) { auto &ChildEdge = *(Node->Successors.begin()); - const unsigned ChildOffset = ChildEdge.second->getOffsetExpr().Offset; - auto &ToMerge = ChildEdge.first; + LTSN *Child = ChildEdge.first; - // Don't collapse if the child has more than one parent - if (not HasAtMostOneParent(ToMerge)) - return false; + revng_log(Log, "Has single child at offset 0. Child: " << Child->ID); + LoggerIndent MoreIndent{ Log }; - // Collapse only if the child is at offset 0 - if (ChildOffset == 0) { - revng_log(Log, "Collapsing " << ToMerge->ID << " into " << Node->ID); - - const unsigned ChildSize = ToMerge->Size; - revng_assert(Node->Size == 0 or Node->Size >= ChildSize); - - // Merge single child into parent. - // mergeNodes resets InterferingInfo of Node, but we're collapsing - // ToMerge that is the only child of Node, so we have to attach ToMerge's - // InterferingInfo to the parent Node that we're preserving. - // This is always correct because the InterferingInfo of a node only - // depend on its children. - auto ChildInterferingInfo = ToMerge->InterferingInfo; - TS.mergeNodes({ /*Into=*/Node, /*From=*/ToMerge }); - Node->Size = ChildSize; - Node->InterferingInfo = ChildInterferingInfo; - - Changed = true; + // If the parent has a size different from the child, bail out. + const unsigned ChildSize = Child->Size; + if (ChildSize != Node->Size) { + revng_log(Log, + "Size mismatch! Node = " << Node->Size + << " Child = " << ChildSize); + break; } + + revng_log(Log, "Collapsing " << Child->ID << " into " << Node->ID); + + // Merge single child into parent. + // mergeNodes resets InterferingInfo of Node, but we're collapsing + // Child that is the only child of Node, so we have to attach Child's + // InterferingInfo to the parent Node that we're preserving. + // This is always correct because the InterferingInfo of a node only + // depend on its children. + auto ChildInterferingInfo = Child->InterferingInfo; + TS.mergeNodes({ /*Into=*/Node, /*From=*/Child }); + Node->InterferingInfo = ChildInterferingInfo; + + Changed = true; } return Changed; @@ -79,13 +72,9 @@ bool CollapseSingleChild::runOnTypeSystem(LayoutTypeSystem &TS) { if (VerifyLog.isEnabled()) revng_assert(TS.verifyDAG()); - for (LTSN *Root : llvm::nodes(&TS)) { - revng_assert(Root != nullptr); - if (not isRoot(Root)) - continue; - - for (LTSN *Node : post_order(Root)) - Changed |= collapseSingle(TS, Node); + for (LTSN *Node : llvm::nodes(&TS)) { + revng_log(Log, "Analyzing Node: " << Node->ID); + Changed |= collapseSingle(TS, Node); } if (VerifyLog.isEnabled()) diff --git a/lib/DataLayoutAnalysis/Middleend/DLADeduplicateUnionFields.cpp b/lib/DataLayoutAnalysis/Middleend/DLADeduplicateUnionFields.cpp index e88834c8c..76827c6fe 100644 --- a/lib/DataLayoutAnalysis/Middleend/DLADeduplicateUnionFields.cpp +++ b/lib/DataLayoutAnalysis/Middleend/DLADeduplicateUnionFields.cpp @@ -374,9 +374,6 @@ bool DeduplicateUnionFields::runOnTypeSystem(LayoutTypeSystem &TS) { if (not isRoot(Root)) continue; - for (LTSN *Node : post_order(NonPointerFilterT(Root))) - TypeSystemChanged |= CollapseSingleChild::collapseSingle(TS, Node); - llvm::SmallVector PostOrderFromRoot; for (LTSN *UnionNode : post_order(NonPointerFilterT(Root))) { if (UnionNode->InterferingInfo != AllChildrenAreInterfering @@ -456,8 +453,8 @@ bool DeduplicateUnionFields::runOnTypeSystem(LayoutTypeSystem &TS) { // to Erased. // BUT: // - postProcessMerge only calls - // - CollapseSingleChild::collapseSingle only removes nodes - // with if these two are safe we're good + // CollapseSingleChild::collapseSingle. + // If these two are safe we're good // - CollapseSingleChild::collapseSingle only removes nodes with // exactly one parent, so it cannot remove nodes that were not // originally children of the union, because if they were they diff --git a/lib/DataLayoutAnalysis/Middleend/DLAStep.h b/lib/DataLayoutAnalysis/Middleend/DLAStep.h index 09cb55a19..3584601ab 100644 --- a/lib/DataLayoutAnalysis/Middleend/DLAStep.h +++ b/lib/DataLayoutAnalysis/Middleend/DLAStep.h @@ -160,7 +160,7 @@ public: CollapseSingleChild() : Step(ID, // Dependencies - {}, + { ComputeUpperMemberAccesses::getID() }, // Invalidated {}) {} diff --git a/tests/unit/DLASteps.cpp b/tests/unit/DLASteps.cpp index 36b944e5d..8935ae32c 100644 --- a/tests/unit/DLASteps.cpp +++ b/tests/unit/DLASteps.cpp @@ -257,37 +257,88 @@ BOOST_AUTO_TEST_CASE(CollapseSingleChild_offsetZero) { dla::LayoutTypeSystem TS; // Build TS - LTSN *Parent = createRoot(TS, 0U); + LTSN *Parent = createRoot(TS, 8U); /*LTSN *Child =*/addInstanceAtOffset(TS, Parent, /*offset=*/0U, /*size=*/8U); // Run step - runStep(TS); + dla::StepManager SM; + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + SM.run(TS); + + // Compress the equivalence classes + dla::VectEqClasses &Eq = TS.getEqClasses(); + Eq.compress(); // Check graph revng_check(TS.getNumLayouts() == 1); checkNode(TS, Parent, 8, InterferingChildrenInfo::Unknown, { 0, 1 }); } +/// Test we don't collapse if the size of the parent is different from the +/// child. +BOOST_AUTO_TEST_CASE(CollapseSingleChild_offsetZero_expected_dontcollapse) { + dla::LayoutTypeSystem TS; + + // Build TS + LTSN *Parent = createRoot(TS, 10U); + LTSN *Child = addInstanceAtOffset(TS, + Parent, + /*offset=*/0U, + /*size=*/8U); + + // Run step + dla::StepManager SM; + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + SM.run(TS); + + // Compress the equivalence classes + dla::VectEqClasses &Eq = TS.getEqClasses(); + Eq.compress(); + + // Check graph + revng_check(TS.getNumLayouts() == 2); + checkNode(TS, Parent, 10, InterferingChildrenInfo::Unknown, { 0 }); + checkNode(TS, Child, 8, InterferingChildrenInfo::Unknown, { 1 }); +} + /// Test the case where there is only one child but with offset > 0 BOOST_AUTO_TEST_CASE(CollapseSingleChild_offsetNonZero) { dla::LayoutTypeSystem TS; // Build TS - LTSN *Parent = createRoot(TS, 0U); + LTSN *Parent = createRoot(TS, 16U); LTSN *Child = addInstanceAtOffset(TS, Parent, /*offset=*/8U, /*size=*/8U); // Run step - runStep(TS); + dla::StepManager SM; + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + SM.run(TS); + + // Compress the equivalence classes + dla::VectEqClasses &Eq = TS.getEqClasses(); + Eq.compress(); // Check graph revng_check(TS.getNumLayouts() == 2); - checkNode(TS, Parent, 0, InterferingChildrenInfo::Unknown, { 0 }); + checkNode(TS, Parent, 16, InterferingChildrenInfo::Unknown, { 0 }); checkNode(TS, Child, 8, InterferingChildrenInfo::Unknown, { 1 }); } @@ -296,11 +347,11 @@ BOOST_AUTO_TEST_CASE(CollapseSingleChild_multiChild) { dla::LayoutTypeSystem TS; // Build TS - LTSN *Parent = createRoot(TS, 0U); + LTSN *Parent = createRoot(TS, 16U); LTSN *Child1 = addInstanceAtOffset(TS, Parent, /*offset=*/0U, - /*size=*/0U); + /*size=*/8U); addInstanceAtOffset(TS, Child1, /*offset=*/0U, @@ -308,18 +359,28 @@ BOOST_AUTO_TEST_CASE(CollapseSingleChild_multiChild) { LTSN *Child2 = addInstanceAtOffset(TS, Parent, /*offset=*/8U, - /*size=*/0U); + /*size=*/8U); addInstanceAtOffset(TS, Child2, /*offset=*/0U, /*size=*/8U); // Run step - runStep(TS); + dla::StepManager SM; + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + SM.run(TS); + + // Compress the equivalence classes + dla::VectEqClasses &Eq = TS.getEqClasses(); + Eq.compress(); // Check graph revng_check(TS.getNumLayouts() == 3); - checkNode(TS, Parent, 0, InterferingChildrenInfo::Unknown, { 0 }); + checkNode(TS, Parent, 16, InterferingChildrenInfo::Unknown, { 0 }); checkNode(TS, Child1, 8, InterferingChildrenInfo::Unknown, { 1, 2 }); checkNode(TS, Child2, 8, InterferingChildrenInfo::Unknown, { 3, 4 }); } @@ -330,26 +391,36 @@ BOOST_AUTO_TEST_CASE(CollapseSingleChild_multiLevel) { dla::LayoutTypeSystem TS; // Build TS - LTSN *Level0 = createRoot(TS, 0U); + LTSN *Level0 = createRoot(TS, 8U); LTSN *Level1 = addInstanceAtOffset(TS, Level0, /*offset=*/0U, - /*size=*/0U); + /*size=*/8U); LTSN *Level2 = addInstanceAtOffset(TS, Level1, /*offset=*/0U, - /*size=*/0U); + /*size=*/8U); LTSN *Level3 = addInstanceAtOffset(TS, Level2, /*offset=*/0U, - /*size=*/0U); + /*size=*/8U); /*LTSN *Level4 = */ addInstanceAtOffset(TS, Level3, /*offset=*/0U, /*size=*/8U); // Run step - runStep(TS); + dla::StepManager SM; + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + revng_check(SM.addStep()); + SM.run(TS); + + // Compress the equivalence classes + dla::VectEqClasses &Eq = TS.getEqClasses(); + Eq.compress(); // Check graph revng_check(TS.getNumLayouts() == 1); @@ -469,6 +540,7 @@ BOOST_AUTO_TEST_CASE(ComputeNonInterferingComponents_basic) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); SM.run(TS); // Compress the equivalence classes @@ -539,6 +611,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_basic) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); @@ -591,6 +664,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_diamond) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); @@ -634,6 +708,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_commonNodeSymmetric) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); @@ -676,6 +751,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_commonNodeAsymmetric) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); @@ -700,7 +776,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_commonNodeAsymmetricCollapse) { LTSN *NodeUnion = createRoot(TS); LTSN *NodeA = addInstanceAtOffset(TS, NodeUnion, /*offset=*/0, /*size=*/0); LTSN *NodeB = addInstanceAtOffset(TS, NodeA, /*offset=*/0, /*size=*/8); - LTSN *NodeC = addInstanceAtOffset(TS, NodeA, /*offset=*/0, /*size=*/0); + LTSN *NodeC = addInstanceAtOffset(TS, NodeA, /*offset=*/0, /*size=*/10); LTSN *NodeD = addInstanceAtOffset(TS, NodeC, /*offset=*/0, /*size=*/8); LTSN *NodeA1 = addInstanceAtOffset(TS, NodeUnion, /*offset=*/0, /*size=*/0); @@ -720,6 +796,7 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_commonNodeAsymmetricCollapse) { revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); + revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); revng_check(SM.addStep()); @@ -731,12 +808,10 @@ BOOST_AUTO_TEST_CASE(DeduplicateUnionFields_commonNodeAsymmetricCollapse) { Eq.compress(); // Check TS - revng_check(TS.getNumLayouts() == 6); - checkNode(TS, NodeUnion, 8, AllChildrenAreInterfering, { 0 }); - checkNode(TS, NodeA, 8, AllChildrenAreInterfering, { 1 }); + revng_check(TS.getNumLayouts() == 5); + checkNode(TS, NodeUnion, 10, AllChildrenAreInterfering, { 0 }); + checkNode(TS, NodeA, 10, AllChildrenAreInterfering, { 1 }); checkNode(TS, NodeB, 8, AllChildrenAreNonInterfering, { 2 }); - checkNode(TS, NodeC, 8, AllChildrenAreNonInterfering, { 3 }); - - checkNode(TS, NodeA1, 8, AllChildrenAreNonInterfering, { 5 }); - checkNode(TS, NodeC1, 8, AllChildrenAreNonInterfering, { 4, 6, 7 }); + checkNode(TS, NodeC, 10, AllChildrenAreNonInterfering, { 3 }); + checkNode(TS, NodeD, 8, AllChildrenAreNonInterfering, { 4, 5, 6, 7 }); }