[Mesa-dev] [PATCH 1/3] r300g: set register classes before interferences

Connor Abbott cwabbott0 at gmail.com
Fri Sep 5 17:59:30 PDT 2014


In commit 567e2769b81863b6dffdac3826a6b729ce6ea37c ("ra: make the p, q
test more efficient") I unknowingly introduced a new requirement to the
register allocator API: the user must set the register class of all
nodes before setting up their interferences, because
ra_add_conflict_list() now uses the classes of the two interfering
nodes. i965 already did this, but r300g was setting up register classes
interleaved with setting up the interference graph. This led to us
calculating the wrong q total, and in certain cases
e78a01d5e6f77e075fe667a0f0ccb10d89c0dd58 (" ra: optimistically color
only one node at a time") made it so that this bug caused a segfault. In
particular, the error occurred if the q total was decremented to 1 below
0 for the last node to be pushed onto the stack.  Since q_total is an
unsigned integer, it overflowed to 0xffffffff, which is what
lowest_q_total happens to be initialzed to. This means that we would
fail the "new_q_total < lowest_q_total" check on line 476 of
register_allocate.c, and so the node would never be pushed onto the
stack, which led to segfaults in ra_select() when we failed to ever give
it a register.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82828
CC: "10.3" <mesa-stable at lists.freedesktop.org>
Signed-off-by: Connor Abbott <cwabbott0 at gmail.com>
---
I think we could get rid of the node_classes array if we call 
ra_alloc_interference_graph() earlier, but I don't know much about this 
code so I just did the minimum amount to fix it.

 src/gallium/drivers/r300/compiler/radeon_pair_regalloc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/gallium/drivers/r300/compiler/radeon_pair_regalloc.c b/src/gallium/drivers/r300/compiler/radeon_pair_regalloc.c
index 936c88d..b854a2f 100644
--- a/src/gallium/drivers/r300/compiler/radeon_pair_regalloc.c
+++ b/src/gallium/drivers/r300/compiler/radeon_pair_regalloc.c
@@ -572,14 +572,16 @@ static void do_advanced_regalloc(struct regalloc_state * s)
 	graph = ra_alloc_interference_graph(ra_state->regs,
 						node_count + s->NumInputs);
 
+	for (node_index = 0; node_index < node_count; node_index++) {
+		ra_set_node_class(graph, node_index, node_classes[node_index]);
+	}
+
 	/* Build the interference graph */
 	for (var_ptr = variables, node_index = 0; var_ptr;
 					var_ptr = var_ptr->Next,node_index++) {
 		struct rc_list * a, * b;
 		unsigned int b_index;
 
-		ra_set_node_class(graph, node_index, node_classes[node_index]);
-
 		for (a = var_ptr, b = var_ptr->Next, b_index = node_index + 1;
 						b; b = b->Next, b_index++) {
 			struct rc_variable * var_a = a->Item;
-- 
1.8.5.2 (Apple Git-48)



More information about the mesa-dev mailing list