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.stell...@amd.com>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=82828
> CC: "10.3" <mesa-sta...@lists.freedesktop.org>
> Signed-off-by: Connor Abbott <cwabbo...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
