On Wed, Aug 10, 2016 at 10:57 AM, Jakub Jelinek <ja...@redhat.com> wrote: > On Wed, Aug 10, 2016 at 08:51:32AM +1000, kugan wrote: >> I see it now. The problem is we are just looking at (-1) being in the ops >> list for passing changed to rewrite_expr_tree in the case of multiplication >> by negate. If we have combined (-1), as in the testcase, we will not have >> the (-1) and will pass changed=false to rewrite_expr_tree. >> >> We should set changed based on what happens in try_special_add_to_ops. >> Attached patch does this. Bootstrap and regression testing are ongoing. Is >> this OK for trunk if there is no regression. > > I think the bug is elsewhere. In particular in > undistribute_ops_list/zero_one_operation/decrement_power. > All those look problematic in this regard, they change RHS of statements > to something that holds a different value, while keeping the LHS. > So, generally you should instead just add a new stmt next to the old one, > and adjust data structures (replace the old SSA_NAME in some ->op with > the new one). decrement_power might be a problem here, dunno if all the > builtins are const in all cases that DSE would kill the old one, > Richard, any preferences for that? reset flow sensitive info + reset debug > stmt uses, or something different? Though, replacing the LHS with a new > anonymous SSA_NAME might be needed too, in case it is before SSA_NAME of a > user var that doesn't yet have any debug stmts.
I'd say replacing the LHS is the way to go, with calling the appropriate helper on the old stmt to generate a debug stmt for it / its uses (would need to look it up here). Richard. > > Jakub
