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

[Tom Stellard]

On Fri, Sep 05, 2014 at 08:59:30PM -0400, Connor Abbott wrote:
> 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.
> 

Reviewed-by: Tom Stellard <thomas.stellard at amd.com>

> 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)
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev