[Bug 106928] When starting a match Rocket League crashes on "Go"

[bugzilla-daemon@freedesktop.org]

[-- Attachment #1.1: Type: text/plain, Size: 3056 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=106928

--- Comment #18 from Miroslav Šustek <sustmidown@centrum.cz> ---
(In reply to Roland Scheidegger from comment #12)
> (In reply to ubizjak from comment #11)
> > The (effectively the same patch as yours) proposed patch would be:
> > 
> > diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp
> > b/src/gallium/drivers/r600/sb/sb_expr.cpp
> > index 7a5d62c8e8..a609d1377f 100644
> > --- a/src/gallium/drivers/r600/sb/sb_expr.cpp
> > +++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
> > @@ -714,6 +714,8 @@ bool expr_handler::fold_assoc(alu_node *n) {
> >  
> >                         n->src.resize(2);
> >                         n->bc.set_op(ALU_OP2_ADD);
> > +                       fold_alu_op2(*n);
> > +                       return true;
> >                 }
> >         } else if (last_arg >= 0) {
> >                 n->src[0] = a->src[last_arg];
> > 
> > WDYT?
> 
> I am not quite convinced it's ok to return true (in fold_alu_op3) if the
> expression hasn't really been folded. You are quite right that just above it
> looks similar, but all other places always return the return value of
> fold_alu_op2 when calling into it from fold_alu_op3.
> (Not saying it isn't correct, just saying I can't tell...)

I guess it is OK. There already is the same pattern in fold_mul_add where it
tries to fold:
> ADD x, MUL(y, z)
into:
> MULADD y, z, x

and when it succeeds, it does:
>
>     fold_alu_op3(*n);
>     return true;

That means that no matter if fold_alu_op3 folds the instruction any further,
fold_mul_add returns true because it converted ADD and MUL into MULADD.

In fold_alu_op2 where fold_mul_add is called, the return value is used in an
early return condition:
> 	if (n.bc.op == ALU_OP2_ADD) {
> 		if (fold_mul_add(&n))
> 			return true;
> 	}

It seems OK to me to return true after calling fold_alu_op2 in fold_alu_op3 as
a sign that the node should not be folded as op3 anymore.

Also, the return value of fold_alu_op* is thrown away anyway here:
> bool expr_handler::try_fold(value* v) {
> 	assert(!v->gvn_source);
> 
> 	if (v->def)
Here expr_handler::try_fold(node* n) calls the fold_alu_op* methods:
> 		try_fold(v->def);
> 
> 	if (v->gvn_source)
> 		return true;
> 
> 	return false;
> }

I like the patch that ubizjak@gmail.com posted:
> diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp b/src/gallium/drivers/r600/sb/sb_expr.cpp
> index 1df78da660..8cbff6f577 100644
> --- a/src/gallium/drivers/r600/sb/sb_expr.cpp
> +++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
> @@ -723,6 +723,8 @@ bool expr_handler::fold_assoc(alu_node *n) {
>  
>                         n->src.resize(2);
>                         n->bc.set_op(ALU_OP2_ADD);
> +                       fold_alu_op2(n);
> +                       return true;
>                 }
>         } else if (last_arg >= 0) {
>                 n->src[0] = a->src[last_arg];

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 4529 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel