From ba50eb4a5586bdb0ddfb6778509752c0f536795f Mon Sep 17 00:00:00 2001 From: Pietro Fezzardi Date: Mon, 6 Jun 2022 15:16:43 +0200 Subject: [PATCH] DLA: revive and rework CollapseSingleChild Step The step now collapse parents with their single child if they are indistinguishable, i.e. if the parent has only that single child, at offset zero, and their size is the same. --- lib/DataLayoutAnalysis/DLAPass.cpp | 1 + lib/DataLayoutAnalysis/DLATypeSystem.cpp | 11 +- .../Middleend/DLACollapseSingleChild.cpp | 77 +++++------ .../Middleend/DLADeduplicateUnionFields.cpp | 7 +- lib/DataLayoutAnalysis/Middleend/DLAStep.h | 2 +- tests/unit/DLASteps.cpp | 121 ++++++++++++++---- 6 files changed, 144 insertions(+), 75 deletions(-) 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 }); }