<html>
<head>
<base href="https://bugs.freedesktop.org/">
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - When starting a match Rocket League crashes on "Go""
href="https://bugs.freedesktop.org/show_bug.cgi?id=106928#c18">Comment # 18</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - When starting a match Rocket League crashes on "Go""
href="https://bugs.freedesktop.org/show_bug.cgi?id=106928">bug 106928</a>
from <span class="vcard"><a class="email" href="mailto:sustmidown@centrum.cz" title="Miroslav Šustek <sustmidown@centrum.cz>"> <span class="fn">Miroslav Šustek</span></a>
</span></b>
<pre>(In reply to Roland Scheidegger from <a href="show_bug.cgi?id=106928#c12">comment #12</a>)
<span class="quote">> (In reply to ubizjak from <a href="show_bug.cgi?id=106928#c11">comment #11</a>)
> > 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...)</span >
I guess it is OK. There already is the same pattern in fold_mul_add where it
tries to fold:
<span class="quote">> ADD x, MUL(y, z)</span >
into:
<span class="quote">> MULADD y, z, x</span >
and when it succeeds, it does:
>
<span class="quote">> fold_alu_op3(*n);
> return true;</span >
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:
<span class="quote">> if (n.bc.op == ALU_OP2_ADD) {
> if (fold_mul_add(&n))
> return true;
> }</span >
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:
<span class="quote">> bool expr_handler::try_fold(value* v) {
> assert(!v->gvn_source);
>
> if (v->def)</span >
Here expr_handler::try_fold(node* n) calls the fold_alu_op* methods:
<span class="quote">> try_fold(v->def);
>
> if (v->gvn_source)
> return true;
>
> return false;
> }</span >
I like the patch that <a href="mailto:ubizjak@gmail.com">ubizjak@gmail.com</a> posted:
<span class="quote">> 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];</span ></pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>