Re: [PR72835] Incorrect arithmetic optimization involving bitfield arguments

[kugan <kugan dot vivekanandarajah at linaro dot org>]

On 10/08/16 18:57, Jakub Jelinek 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.

Hi Jakub,

This is the patch I have now (not full tested yet). This is along what 
you described above. I think I have to handle all the LHS in 
zero_one_operation too, I will wait for the feedback before working on it.

Thanks,
Kugan

Attachment: pr72835_3.txt
Description: Text document