Re: [PATCH] fix possible FP reassociation problem in simplify-rtx.c

[Paolo Bonzini <paolo dot bonzini at lu dot unisi dot ch>]

> > -  /* If we only have two operands, we can't do anything.  */
> > -  if (n_ops <= 2 && !force)
> > +  gcc_assert (n_ops >= 2);
> > +  if (!canonicalized)
> >     return NULL_RTX;
> >    
> I'll let you decide whether its necessary to have a gcc_assert here.
> The initial value of n_ops is two, and its only ever incremented,
> (upon encounting a binary operator in a single RTX).  i.e. it can't
> overflow and can't ever decrease.  The previous condition was just
> a defensive form of "n_ops == 2".
>  
I usually am quite liberal in asserts, if the condition is verified 
after a loop that is quite long, as in this case.  Actually I added the 
assert because the "n_ops <= 2" made me wonder how n_ops could ever be < 2.

> Was this change to disable floating point reassociation inspired
> by a real test case, or from inspection of the code?
>  
From inspection (hence the "possible" in the topic).  I was looking at 
whether the call to simplify_plus_minus in simplify_gen_binary is really 
necessary, because combine calls simplify_binary_operation only.

> It might be
> reasonable to continue to allow reassociation of FP operations with
> flag_unsafe_math_optimizations.
Yes.  I'll try it after I flush some other patches I have here.

> Ok for mainline with the correction to the typo.
>  
Thanks for the detailed review.

Paolo