Comment # 12
on bug 106928
from Roland Scheidegger
(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...) > On a side note, maybe -D_GLIBCXX_ASSERTIONS should be added to mesa > testsuite. 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. I was actually going to suggest to enable -D_GLIBCXX_DEBUG for debug builds always, but it wouldn't work in general (due to not being able to link anything which hasn't been compiled with it, for instance llvm). So using -D_GLIBCXX_ASSERTIONS instead (which should be link-compatible) looks like a good idea to me, albeit it didn't do anything for me (too old libstdc++ I suppose). I'm also not quite sure which issues it actually catches (vs the DEBUG one) but even if it just catches some that should be a plus...
You are receiving this mail because:
- You are the assignee for the bug.
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
