On Fri, Jan 15, 2016 at 11:38 AM, Alan Lawrence <alan.lawre...@foss.arm.com> wrote: > On 15/01/16 10:07, Richard Biener wrote: >> >> On Fri, Jan 15, 2016 at 10:47 AM, Alan Lawrence <alan.lawre...@arm.com> >> wrote: >>> >>> On Thu, Jan 14, 2016 at 12:30 PM, Richard Biener >>> <richard.guent...@gmail.com> wrote: >>> >>>> >>>> The vuse test is not necessary >>>> >>>>> + && (gimple_assign_rhs_code (def) == SSA_NAME >>>>> + || is_gimple_min_invariant (gimple_assign_rhs1 >>>>> (def)))) >>>> >>>> >>>> and the is_gimple_min_invariant (rhs1) test is not sufficient if you >>>> consider - (-INT_MIN) with -ftrapv for example. >>> >>> >>> Thanks, I didn't realize gimple_min_invariant would allow such cases. >> >> >> Well, the invariant would be -INT_MIN but gimple_assign_rhs_code (def) >> would >> be NEGATE_EXPR. Basically you forgot about unary operators. > > > Hmm, shouldn't those have get_gimple_rhs_class(gimple_assign_rhs_code(stmt)) > == GIMPLE_UNARY_RHS, rather than GIMPLE_SINGLE_RHS as checked for by > gimple_assign_single_p?
Doh, of course. > If SINGLE_RHS includes unary operators, the new version of the patch is as > flawed as the previous, in that it drops the unary operator altogether. > > --Alan
