[PATCH] Assert in PBQP: A conservatively allocatable node should never be spilled.

[Arnaud A. de Grandmaison]

I agree we should have this assert in place, especially for the time being
where the assumption that a node is conservatively allocatable does not seem
to hold.

 

The patch looks good to me, but I leave the final word to Lang.

 

Cheers,

--

Arnaud

 

From: Jonas Paulsson [mailto:jonas.paulsson at ericsson.com] 
Sent: 06 February 2015 12:26
To: Lang Hames; Arnaud De Grandmaison
Cc: llvm-commits at cs.uiuc.edu
Subject: [PATCH] Assert in PBQP: A conservatively allocatable node should
never be spilled.

 

Hi,

 

I have polished up my patch for the assert regarding conservatively
allocatable nodes. I find this assert to be desirable, as there is an
implementation error somewhere if any of these nodes are spilled, but that
is something that may not be detected, as most often the output is still
legal. InlineSpiller only complains if a spilled register is spilled again,
for instance.

 

Can I commit this or do you have any comments / opinion on this?

 

/Jonas Paulsson

 

Patch:

 

>From 64ecd655faa1a1d133247fefc545472c042e6368 Mon Sep 17 00:00:00 2001

From: Jonas Paulsson <jonas.paulsson at ericsson.com>

Date: Wed, 28 Jan 2015 08:30:35 +0100

Subject: [PATCH] Assert in PBQP: A conservatively allocatable node should

never be spilled.

 

---

include/llvm/CodeGen/RegAllocPBQP.h | 16 ++++++++++++++++

1 file changed, 16 insertions(+)

 

diff --git a/include/llvm/CodeGen/RegAllocPBQP.h
b/include/llvm/CodeGen/RegAllocPBQP.h

index cfa6160..140abed8 100644

--- a/include/llvm/CodeGen/RegAllocPBQP.h

+++ b/include/llvm/CodeGen/RegAllocPBQP.h

@@ -308,6 +308,13 @@ public:

     setup();

     S = backpropagate(G, reduce());

     G.unsetSolver();

+

+#ifndef NDEBUG

+    for (auto NId : WasConservativelyAllocables)

+      assert (S.getSelection(NId) != PBQP::RegAlloc::getSpillOptionIdx() &&

+              "A conservatively allocatable node was spilled!");

+#endif

+

     return S;

   }

 

@@ -464,6 +471,9 @@ private:

         NodeStack.push_back(NId);

         G.disconnectAllNeighborsFromNode(NId);

 

+#ifndef NDEBUG

+        WasConservativelyAllocables.insert(NId);

+#endif

       } else if (!NotProvablyAllocatableNodes.empty()) {

         NodeSet::iterator NItr =

           std::min_element(NotProvablyAllocatableNodes.begin(),

@@ -497,6 +507,12 @@ private:

   NodeSet OptimallyReducibleNodes;

   NodeSet ConservativelyAllocatableNodes;

   NodeSet NotProvablyAllocatableNodes;

+

+#ifndef NDEBUG

+  // A set of nodes to that were conservatively allocatable and are

+  // thus never supposed to be spilled.

+  NodeSet WasConservativelyAllocables;

+#endif

};
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150206/be250d88/attachment.html>