[llvm] r290950 - Fix for InlineSpiller accessing not updated dom tree base information.

Bjorn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 4 01:41:56 PST 2017


Author: bjope
Date: Wed Jan  4 03:41:56 2017
New Revision: 290950

URL: http://llvm.org/viewvc/llvm-project?rev=290950&view=rev
Log:
Fix for InlineSpiller accessing not updated dom tree base information.

Summary:
The InlineSpiller was accessing the DominatorTreeBase directly
through the public data member DT in the MachineDominatorTree.
This is not a good idea as the "cached" information in
SplitCriticalEdges is not applied before the access.
The DominatorTreeBase must be accessed through the member
function getBase() in MachineDominatorTree.

The fault was introduced in r266162.

I think the public data member DT in the MachineDominatorTree
should have been made private in the original code (r215576)
that introduced the concept of lazily updating the
MachineDominatorTree information from
MachineBasicBlock::SplitCriticalEdge().

Patch by Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>

Reviewers: wmi, qcolombet

Subscribers: llvm-commits, bjope, uabelho

Differential Revision: https://reviews.llvm.org/D27983

Modified:
    llvm/trunk/include/llvm/CodeGen/MachineDominators.h
    llvm/trunk/lib/CodeGen/InlineSpiller.cpp

Modified: llvm/trunk/include/llvm/CodeGen/MachineDominators.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineDominators.h?rev=290950&r1=290949&r2=290950&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineDominators.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineDominators.h Wed Jan  4 03:41:56 2017
@@ -59,6 +59,9 @@ class MachineDominatorTree : public Mach
   /// such as BB == elt.NewBB.
   mutable SmallSet<MachineBasicBlock *, 32> NewBBs;
 
+  /// The DominatorTreeBase that is used to compute a normal dominator tree
+  DominatorTreeBase<MachineBasicBlock>* DT;
+
   /// \brief Apply all the recorded critical edges to the DT.
   /// This updates the underlying DT information in a way that uses
   /// the fast query path of DT as much as possible.
@@ -68,7 +71,6 @@ class MachineDominatorTree : public Mach
 
 public:
   static char ID; // Pass ID, replacement for typeid
-  DominatorTreeBase<MachineBasicBlock>* DT;
 
   MachineDominatorTree();
 

Modified: llvm/trunk/lib/CodeGen/InlineSpiller.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/InlineSpiller.cpp?rev=290950&r1=290949&r2=290950&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/InlineSpiller.cpp (original)
+++ llvm/trunk/lib/CodeGen/InlineSpiller.cpp Wed Jan  4 03:41:56 2017
@@ -1124,7 +1124,7 @@ void HoistSpillHelper::rmRedundantSpills
   // earlier spill with smaller SlotIndex.
   for (const auto CurrentSpill : Spills) {
     MachineBasicBlock *Block = CurrentSpill->getParent();
-    MachineDomTreeNode *Node = MDT.DT->getNode(Block);
+    MachineDomTreeNode *Node = MDT.getBase().getNode(Block);
     MachineInstr *PrevSpill = SpillBBToSpill[Node];
     if (PrevSpill) {
       SlotIndex PIdx = LIS.getInstructionIndex(*PrevSpill);
@@ -1132,9 +1132,9 @@ void HoistSpillHelper::rmRedundantSpills
       MachineInstr *SpillToRm = (CIdx > PIdx) ? CurrentSpill : PrevSpill;
       MachineInstr *SpillToKeep = (CIdx > PIdx) ? PrevSpill : CurrentSpill;
       SpillsToRm.push_back(SpillToRm);
-      SpillBBToSpill[MDT.DT->getNode(Block)] = SpillToKeep;
+      SpillBBToSpill[MDT.getBase().getNode(Block)] = SpillToKeep;
     } else {
-      SpillBBToSpill[MDT.DT->getNode(Block)] = CurrentSpill;
+      SpillBBToSpill[MDT.getBase().getNode(Block)] = CurrentSpill;
     }
   }
   for (const auto SpillToRm : SpillsToRm)
@@ -1209,7 +1209,7 @@ void HoistSpillHelper::getVisitOrders(
   // Sort the nodes in WorkSet in top-down order and save the nodes
   // in Orders. Orders will be used for hoisting in runHoistSpills.
   unsigned idx = 0;
-  Orders.push_back(MDT.DT->getNode(Root));
+  Orders.push_back(MDT.getBase().getNode(Root));
   do {
     MachineDomTreeNode *Node = Orders[idx++];
     const std::vector<MachineDomTreeNode *> &Children = Node->getChildren();




More information about the llvm-commits mailing list