Comme=
nt # 11
on bug 10692=
8
from ubizjak@gmail.co=
m
(In reply to Roland Scheidegger from comment #9)
> (In reply to ubizjak from comment #7)
> > Please configure the build with:
> >=20
> > CXXFLAGS=3D"-Wp,-D_GLIBCXX_ASSERTIONS" ./autogen.sh
>=20
> That didn't do anything neither. However I figured out the problem mor=
e or
> less in the code, and some googling said that using -D_GLIBCXX_DEBUG s=
hould
> make it trigger reliably, and indeed it does...
Great ;)
> The issue is that (you already showed that actua=
lly)
> src =3D std::vector of length 2, capacity 3 =3D {0x7f94d905d110, 0x7f9=
4d905cf70}}
> And trying to access element src[2].
> There's an early exit in the function if src.size() is < 3. Since t=
his
> didn't hit, apparently fold_assoc() resized the vector. And indeed it =
can do
> that (there's an explicit n->src.resize(2) somewhere, and it would =
still
> return false in this case).
>=20
> I think something like this should do:
> diff --git a/src/gallium/drivers/r600/sb/sb_expr.cpp
> b/src/gallium/drivers/r600/sb/sb_expr.cpp
> index 1df78da660..c77b9f2d7d 100644
> --- a/src/gallium/drivers/r600/sb/sb_expr.cpp
> +++ b/src/gallium/drivers/r600/sb/sb_expr.cpp
> @@ -945,6 +945,8 @@ bool expr_handler::fold_alu_op3(al=
u_node& n) {
> if (!sh.safe_math && (n.bc.op_ptr->flags & AF_M=
_ASSOC)) {
> if (fold_assoc(&n))
> return true;
> + else if (n.src.size() < 3)
> + return fold_alu_op2(n);
> }
>=20=20
> value* v0 =3D n.src[0]->gvalue();
I wonder if we should fix expr_handler::fold_assoc instead. Digging through=
the
code, fold_assoc is called only from expr_handler::fold_alu_op2 and
expr_handler::fold_alu_op3. When true is returned, it triggers an early exit
from expr_handler::fold_alu_op{2,3} functions.
The part we are looking for in expr_handler::fold_assoc is:
} else { // MULADD =3D> ADD
n->src[0] =3D n->src[2];
n->bc.src[0] =3D n->bc.src[2];
n->src[1] =3D sh.get_const_value(cr);
memset(&n->bc.src[1], 0, sizeof(bc_alu_src));
n->src.resize(2);
n->bc.set_op(ALU_OP2_ADD);
}
So, let's call fold_alu_op2 here and return true to trigger early exit in
expr_handler::fold_alu_op3. This is what the code a couple of lines above t=
he
presented code does when an operand degenerates to mov.
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 >=3D 0) {
n->src[0] =3D a->src[last_arg];
WDYT?
On a side note, maybe -D_GLIBCXX_ASSERTIONS should be added to mesa testsui=
te.
This is the flag that Fedora 28 builds use by default now, so it would be
beneficial to catch these bugs early in the development cycle, before they
reach users.